From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#68579: [PATCH] Support a local repo as URL in treesit-language-source-alist Date: Fri, 19 Jan 2024 10:33:45 +0200 Message-ID: <83frytwwly.fsf@gnu.org> References: <0ffc2f474cf84ed3c63aa82091c967807e5ca0e6.camel@yandex.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="2907"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 68579@debbugs.gnu.org To: Konstantin Kharlamov , Yuan Fu Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jan 19 09:34:21 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rQkKT-0000Y1-1e for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 19 Jan 2024 09:34:21 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rQkKA-0005Lz-Ro; Fri, 19 Jan 2024 03:34:02 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rQkK8-0005LE-BV for bug-gnu-emacs@gnu.org; Fri, 19 Jan 2024 03:34:00 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rQkK8-00033R-2o for bug-gnu-emacs@gnu.org; Fri, 19 Jan 2024 03:34:00 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rQkKA-0003g6-4p for bug-gnu-emacs@gnu.org; Fri, 19 Jan 2024 03:34:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 19 Jan 2024 08:34:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 68579 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 68579-submit@debbugs.gnu.org id=B68579.170565323814124 (code B ref 68579); Fri, 19 Jan 2024 08:34:02 +0000 Original-Received: (at 68579) by debbugs.gnu.org; 19 Jan 2024 08:33:58 +0000 Original-Received: from localhost ([127.0.0.1]:57445 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rQkK6-0003fk-3g for submit@debbugs.gnu.org; Fri, 19 Jan 2024 03:33:58 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:38472) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rQkK4-0003fV-N3 for 68579@debbugs.gnu.org; Fri, 19 Jan 2024 03:33:57 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rQkJx-00032Y-3k; Fri, 19 Jan 2024 03:33:49 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=AalonTr1ksMzgLpvhShB8vpMP5Q58DyXqaqQ5M7R6wU=; b=LfLqi2Jzf3E9NTFe92C8 nXfp+iJRyZ6ui/+HFNG5yy6kz47w+8V1eFBXINDn7adI2tdWzRAY9SrYsuiFwIQY3iL4Prxw9mlyf xgtJi7yVHWeWn6N7plefAEjlIt1bkUpgcwXYTwXpI8gX/mAJaZ9kFh+Z7ggnaKsrqiIGZrn/3Si+H OtVOOPoN2mm/tQXkMGiwgJ1x+wdKYKYojgYcTQ/hvCdWu0GLJaZZiMz3VyBo7Jh5nOraC5ZgWzPPV orjG/1L/EATLVXI9uxHVTcWe+MBEkdEc0P4gmBd0UmWArV54r8SKdhgXoktrqeBu7HRf0KUK2oaQh MO8yAmCmupTqJQ==; In-Reply-To: <0ffc2f474cf84ed3c63aa82091c967807e5ca0e6.camel@yandex.ru> (message from Konstantin Kharlamov on Fri, 19 Jan 2024 11:08:00 +0300) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:278486 Archived-At: > From: Konstantin Kharlamov > Date: Fri, 19 Jan 2024 11:08:00 +0300 > > So what this patch does is it adds detection of a full path to the > treesit-language-source-alist processing to make sure we avoid cloning > the repo in that case and just proceed building it. Thanks. Some comments below, and I added Yuan to the discussion. > From c9de59a39566a5df46aa39ef93348487bb2948aa Mon Sep 17 00:00:00 2001 > From: Konstantin Kharlamov > Date: Fri, 19 Jan 2024 10:33:47 +0300 > Subject: [PATCH] Support a local repo as URL in treesit-language-source-alist > > Sometimes people may need to bisect to find specific revision in a > grammar repo. In this case they'd want to point the URL to the local > repo to avoid cloning it on every rebuild. So add support for full > path in treesit-language-source-alist. > > * lisp/treesit.el (treesit--install-language-grammar-1): test if URL > starts with / meaning that the URL is a local path. Then if it is, > avoid cloning the repo and removing the path on success. > (treesit--git-clone-repo): factor out the code for cloning to a separate > function. > (treesit--git-checkout-branch): a helper to checkout the revision for > cases where we didn't clone the repo but want it to point the > revision. These descriptions should be full sentences: start with a capital letter and end in a period. Also, please leave two spaces between sentences. > Only LANG and URL are mandatory. LANG is the language symbol. > -URL is the Git repository URL for the grammar. > +URL is the Git repository URL or full path for the grammar. GNU Coding Standards frown on using "path" for anything but PATH-style directory lists. In this case, you mean "directory name". > +(defun treesit--git-checkout-branch (repo-path revision) > + "Checkout `revision' in a repo located in `repo-path'" References to arguments in doc strings should be capitalized and not quoted: REVISION and REPO-PATH (which should be renamed to REPO-DIR). > +(defun treesit--git-clone-repo (url revision workdir) > + "Clone repo pointed by `url' at commit `revision' to `workdir'" Likewise. Also, please document in the doc string what happens if REVISION is nil. > + ;; don't clone if url is a local path Comments should be complete sentences: begin with a capital letter and end with a period. > + (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. > - (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? > + (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)) ? > - (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? Finally, this needs a NEWS entry.