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

From: Eli Zaretskii <[email-masked]>
Date: Fri, 19 Jan 2024 13:46:54 +0200

> From: Konstantin Kharlamov <[email-masked]>
> Cc: [email-masked]
> 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…?

I'm talking about distinguishing between a URL and a local file name.
Are we guaranteed to get an absolute file name there?  If not, how do
you know that something like "http://foo.bar" cannot be a local file
name?

> > > -         (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 😅

The expand-file-name was there, yes.  But why do you think we should
avoid calling expand-file-name if URL is a local file name?

> > > -      (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.

But you still clone from it into workdir, no?  treesit--git-clone-repo
invokes "git clone" in both cases, according to my reading of the patch.