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
next prev parent 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
List information: https://www.gnu.org/software/emacs/
* 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 public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).