GNU bug report logs - #68579
[PATCH] Support a local repo as URL in treesit-language-source-alist

Previous Next

Full log

Message #11 received at 68579 <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Yuan Fu <casouri <at> gmail.com>
Cc: 68579 <at> debbugs.gnu.org
Subject: Re: bug#68579: [PATCH] Support a local repo as URL in
 treesit-language-source-alist
Date: Fri, 19 Jan 2024 11:57:48 +0300

On Fri, 2024-01-19 at 10:33 +0200, Eli Zaretskii wrote:
[…]

Thank you!

> 
> > +         (url-is-path (string-prefix-p "/" url))
> 
> "Path" used wrongly again.  Also, the string-prefix-p test is too
> naïve and unportable.  I think file-name-absolute-p is a better test
> (assuming we expect an absolute file name there), perhaps also
> augmented by file-accessible-directory-p.

I would presume if the directory inaccessible some later commands such
as `git checkout` will fail anyway, so no point in adding the `file-
accessible-directory-p` check on Emacs side…?

> > -         (workdir (expand-file-name "repo"))
> > +         (workdir (if url-is-path url (expand-file-name "repo")))
> 
> Not sure about this hunk: why do we not need to expand-file-name if
> URL is not a local directory but a real URL?

Idk, that was there 😅 But yeah, I can remove if it's not needed. I
presume if it's really needed, there needs to be an explanation comment

> > +          (if url-is-path
> > +              (when revision
> > +                (treesit--git-checkout-branch url revision))
> 
> Isn't the above equivalent to
> 
>              (and url-is-path revision
>                   (treesit--git-checkout-branch url revision))
> 
> ?

Good point!

> > -      (when (file-exists-p workdir)
> > +      (when (and (not url-is-path) (file-exists-p workdir))
> >          (delete-directory workdir t)))))
> 
> Why?  Does workdir have different semantics in these two use cases?
> Isn't it the directory where we cloned the repository?

When an absolute path is passed as URL, that means the user have cloned
the repo, not us. So we do not want to remove the directory. This is a
different semantics from the case where we created a temporary clone
ourselves, and want to get rid of the temporary directory after
everything is done.

I can add a comment explaining that there

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.