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

Previous Next

Package: emacs;

Reported by: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>

Date: Fri, 19 Jan 2024 08:09:02 UTC

Severity: wishlist

Tags: patch

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 68579 <at> debbugs.gnu.org
Subject: bug#68579: [PATCH] Support a local repo as URL in treesit-language-source-alist
Date: Fri, 19 Jan 2024 17:33:03 +0300
On Fri, 2024-01-19 at 13:46 +0200, Eli Zaretskii wrote:
> > From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
> > Cc: 68579 <at> debbugs.gnu.org
> > 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?

Ah, gotcha!

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

Ah, sorry, I got that reversed. Yeah, it makes sense calling that for a
local filename. I was wondering why was it there for a URL, which is
partially why I read you wrong.

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

No, the `treesit--git-clone-repo` is located on the "else" branch of
the `(if url-is-path`. That is, we do not call it when it's a local
path




This bug report was last modified 1 year and 163 days ago.

Previous Next


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