all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
To: Eli Zaretskii <eliz@gnu.org>, Yuan Fu <casouri@gmail.com>
Cc: 68579@debbugs.gnu.org
Subject: bug#68579: [PATCH] Support a local repo as URL in treesit-language-source-alist
Date: Fri, 19 Jan 2024 11:57:48 +0300	[thread overview]
Message-ID: <7010aa960be524d13f4dd96a1c7ea9102b580adf.camel@yandex.ru> (raw)
In-Reply-To: <83frytwwly.fsf@gnu.org>

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





  reply	other threads:[~2024-01-19  8:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-19  8:08 bug#68579: [PATCH] Support a local repo as URL in treesit-language-source-alist Konstantin Kharlamov
2024-01-19  8:33 ` Eli Zaretskii
2024-01-19  8:57   ` Konstantin Kharlamov [this message]
2024-01-19 11:46     ` Eli Zaretskii
2024-01-19 14:33       ` Konstantin Kharlamov
2024-01-19 15:06         ` Eli Zaretskii
2024-01-19 16:06           ` Konstantin Kharlamov
2024-01-19 16:26             ` Eli Zaretskii
2024-01-20 12:00               ` bug#68579: [PATCH v2] " Konstantin Kharlamov
2024-01-20 12:04                 ` Konstantin Kharlamov
2024-01-20 11:56 ` Konstantin Kharlamov
2024-01-27  9:37   ` Eli Zaretskii
2024-01-27 18:53     ` Konstantin Kharlamov
2024-01-27 19:21       ` Eli Zaretskii
2024-02-01 10:06     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7010aa960be524d13f4dd96a1c7ea9102b580adf.camel@yandex.ru \
    --to=hi-angel@yandex.ru \
    --cc=68579@debbugs.gnu.org \
    --cc=casouri@gmail.com \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.