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

[Konstantin Kharlamov]

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