From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: "Basil L. Contovounesios" Newsgroups: gmane.emacs.devel Subject: Re: RFC: Automatic setup for bug-reference-mode Date: Sun, 14 Jun 2020 13:13:57 +0100 Message-ID: <87blllx2t6.fsf@tcd.ie> References: <87r1uihtsu.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="62254"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Stefan Monnier To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Jun 14 14:14:37 2020 Return-path: Envelope-to: ged-emacs-devel@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 1jkRX2-000G29-SF for ged-emacs-devel@m.gmane-mx.org; Sun, 14 Jun 2020 14:14:36 +0200 Original-Received: from localhost ([::1]:59850 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jkRX1-00024f-Sk for ged-emacs-devel@m.gmane-mx.org; Sun, 14 Jun 2020 08:14:35 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:40106) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jkRWX-0001fj-06 for emacs-devel@gnu.org; Sun, 14 Jun 2020 08:14:05 -0400 Original-Received: from mail-wr1-x42b.google.com ([2a00:1450:4864:20::42b]:38477) by eggs.gnu.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jkRWU-0006pO-LG for emacs-devel@gnu.org; Sun, 14 Jun 2020 08:14:04 -0400 Original-Received: by mail-wr1-x42b.google.com with SMTP id e1so14317258wrt.5 for ; Sun, 14 Jun 2020 05:14:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=RRrZtLTVFAAKU77uTkwuHiA9Y3pqjFHY2CMx3P2zOl0=; b=CWwVfElWyuLYiCfqmHFtEctspZ0LllhEcs/ZqWYUh+wiJ7t8r5ITyJq+5QKlq90uMz 0OQhkUmqpFe1UEjYOUJaycdqY28tyJ9E135oa6lG2An+7t0twmR+HDT0hvxCaIpKEJJf XFjnOBhBUg6JQO5HfVMe6TknzgvkJ5ImmDNMIGkUVGa2NE/eUxuLeCnnwVj7n0MWqp9d wdFXfZjezvtK2DIK8aCtXyg3WXUAwtgdQKAFHYQ8lhCDAt9hqSjCSCigCLoV/3dQ4ZNO jHQ5TuCe5Ai4XdsO/qBz1vlbkC51f8UlNtLw9QiCvUPL4hlMWhTQqJoVESOGYT97YCVj UkPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=RRrZtLTVFAAKU77uTkwuHiA9Y3pqjFHY2CMx3P2zOl0=; b=nEd20NhJgj9JWwhjKTaxf9ebk2cwCKt0ZKpSiob/+3BXhczwbIstdKZVhiD2OCbd1d RKP0113bU3W2npZUueeYGnrHUiqjoXzOF8vmrUkVQhP749KlcX4T/izDSiQsWsu3b9yJ PKmb0iuHsR1XM2OoSkAVzRLLnh2wWm0ZdYYP8ic2fu0ma6uojOKMx4NVWROwTQF8AMP7 d8m6f2lIB41FPY9AZ7m6bMjpmu5c/RtqyIwk4lWUuoQnfYjdv0rBfYlgpcNRrNlCPYGx if1R+IquHjXk4+e7Y0RHxr9amUbLZ68gU+q6wuC3niiad3JOqZCfJdRYJPuj6ktFx+xF OzVQ== X-Gm-Message-State: AOAM532yEBOvsIQTTQ8LOPEKdVlBe9SdwT7VXhhcpiuHAyv9pV0rsQWH YH4SeiHKhHmNf3Hriyp/V77XA8cKEOkRVQ== X-Google-Smtp-Source: ABdhPJyexgT4LZw0LP+cfLLEHK/ZrkzOdfpUaibEzRkseXxvFAENdAFsMN57g9g0LYuyWoskfAe/GQ== X-Received: by 2002:adf:d851:: with SMTP id k17mr23729653wrl.30.1592136840026; Sun, 14 Jun 2020 05:14:00 -0700 (PDT) Original-Received: from localhost ([2a02:8084:20e2:c380:1f68:7ff5:120d:64e]) by smtp.gmail.com with ESMTPSA id b8sm20156251wrs.36.2020.06.14.05.13.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 14 Jun 2020 05:13:59 -0700 (PDT) In-Reply-To: <87r1uihtsu.fsf@gnu.org> (Tassilo Horn's message of "Sun, 14 Jun 2020 11:37:37 +0200") Received-SPF: none client-ip=2a00:1450:4864:20::42b; envelope-from=contovob@tcd.ie; helo=mail-wr1-x42b.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:252225 Archived-At: Tassilo Horn writes: > Attach is my first attempt at doing so and I'd welcome comments. Thanks, just some minor comments from me. [...] > modified lisp/progmodes/bug-reference.el > @@ -60,6 +60,7 @@ bug-reference-url-format > you need to add a `bug-reference-url-format' property to it: > \(put \\='my-bug-reference-url-format \\='bug-reference-url-format t) > so that it is considered safe, see `enable-local-variables'.") > +(make-variable-buffer-local 'bug-reference-url-format) Nit: You can use defvar-local instead. > ;;;###autoload > (put 'bug-reference-url-format 'safe-local-variable > @@ -75,6 +76,7 @@ bug-reference-bug-regexp > :type 'regexp > :version "24.3" ; previously defconst > :group 'bug-reference) > +(make-variable-buffer-local 'bug-reference-bug-regexp) Nit: You can use ':local t' instead. > ;;;###autoload > (put 'bug-reference-bug-regexp 'safe-local-variable 'stringp) > @@ -139,6 +141,139 @@ bug-reference-push-button > (when url > (browse-url url)))))) > > +(defcustom bug-reference-setup-functions nil > + "A list of function for setting up bug-reference mode. Nit: Either "Bug-Reference Mode" or "`bug-reference-mode'". > +A setup function should return non-nil if it set > +`bug-reference-bug-regexp' and `bug-reference-url-format' > +appropiately for the current buffer. The functions are called in "appropriately" > +sequence stopping as soon as one signalled a successful setup. The Emacs sources predominantly use the spelling "signaled". > +They are only called if the two variables aren't set already, > +e.g., by a local variables section. > + > +Also see `bug-reference-default-setup-functions'. > + > +The `bug-reference-setup-functions' take preference over "take precedence" > +`bug-reference-default-setup-functions', i.e., they are > +called before the latter." > + :type '(list function) Either 'hook or '(repeat function). > + :version "28.1" > + :group 'bug-reference) > + > +(defun bug-reference-try-setup-from-vc () > + "Try setting up `bug-reference-bug-regexp' and > +`bug-reference-url-format' from the version control system of the > +current file." Nit: Please follow the recommendation in "(elisp) Documentation Tips" of fitting the first sentence on a single line. > + (when (buffer-file-name) > + (let* ((backend (vc-responsible-backend (buffer-file-name) t)) Nit: Any reason not to use the buffer-local variable buffer-file-name instead? > + (url (pcase backend > + ('Git (string-trim This needs (eval-when-compile (require 'subr-x)). > + (shell-command-to-string > + "git ls-remote --get-url")))))) Doesn't VC provide a robust way to get output from Git? > + (cl-flet ((maybe-set (url-rx bug-rx bug-url-fmt) > + (when (string-match url-rx url) > + (setq bug-reference-bug-regexp bug-rx) > + (setq bug-reference-url-format > + (if (functionp bug-url-fmt) > + (funcall bug-url-fmt) > + bug-url-fmt))))) > + (when (and url > + ;; If there's a space in the url, it's propably an > + ;; error message. > + (not (string-match-p "[[:space:]]" url))) > + (or > + ;; GNU projects on savannah. FIXME: Only a fraction of > + ;; them uses debbugs. > + (maybe-set "git\\.\\(sv\\|savannah\\)\\.gnu\\.org:" ^^^ Nit: Can this be a shy group? > + "\\([Bb]ug ?#?\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)" > + "https://debbugs.gnu.org/%s") > + ;; GitHub projects. Here #17 may refer to either an issue > + ;; or a pull request but visiting the issue/17 web page > + ;; will automatically redirect to the pull/17 page if 17 is > + ;; a PR. Explicit user/project#17 links to possibly > + ;; different projects are also supported. > + (maybe-set > + "[/@]github.com[/:]\\([.A-Za-z0-9_/-]+\\)\\.git" > + "\\([.A-Za-z0-9_/-]+\\)?\\(?:#\\)\\([0-9]+\\)" ^^^^^^^^^ Nit: Why is this in a group? > + (lambda () > + (let ((ns-project (match-string 1 url))) > + (lambda () > + (concat "https://github.com/" > + (or > + ;; Explicit user/proj#18 link. > + (match-string 1) > + ns-project) > + "/issues/" > + (match-string 2)))))) > + ;; GitLab projects. Here #18 is an issue and !17 is a > + ;; merge request. Explicit namespace/project#18 references > + ;; to possibly different projects are also supported. > + (maybe-set > + "[/@]gitlab.com[/:]\\([.A-Za-z0-9_/-]+\\)\\.git" > + "\\(?1:[.A-Za-z0-9_/-]+\\)?\\(?3:#\\|!\\)\\(?2:[0-9]+\\)" Nit: Isn't this the same as "\\(?3:[!#]\\)"? > + (lambda () > + (let ((ns-project (match-string 1 url))) > + (lambda () > + (concat "https://gitlab.com/" > + (or (match-string 1) > + ns-project) > + "/-/" > + (if (string= (match-string 3) "#") > + "issues/" > + "merge_requests/") > + (match-string 2)))))))))))) > + > +(defun bug-reference-try-setup-from-gnus () > + (when (and (memq major-mode '(gnus-summary-mode gnus-article-mode)) > + (boundp 'gnus-newsgroup-name) > + gnus-newsgroup-name) (and (derived-mode-p 'gnus-summary-mode 'gnus-article-mode) (bound-and-true-p gnus-newsgroup-name)) or (and (derived-mode-p 'gnus-mode) (bound-and-true-p gnus-newsgroup-name)) [...] > +(defun bug-reference--init () > + "Initialize `bug-reference-mode'." > + (progn Nit: Isn't this progn redundant? > + ;; Automatic setup only if the variables aren't already set, e.g., > + ;; by a local variables section in the file. > + (unless (and bug-reference-bug-regexp > + bug-reference-url-format) > + (or > + (with-demoted-errors > + "Error while running bug-reference-setup-functions: %S" > + (run-hook-with-args-until-success > + 'bug-reference-setup-functions)) > + (with-demoted-errors > + "Error while running bug-reference-default-setup-functions: %S" > + (run-hook-with-args-until-success > + 'bug-reference-default-setup-functions)))) > + (jit-lock-register #'bug-reference-fontify))) [...] > ;;;###autoload > -(defun vc-responsible-backend (file) > +(defun vc-responsible-backend (file &optional no-error) > "Return the name of a backend system that is responsible for FILE. > > If FILE is already registered, return the > @@ -967,7 +967,10 @@ vc-responsible-backend > > Note that if FILE is a symbolic link, it will not be resolved -- > the responsible backend system for the symbolic link itself will > -be reported." > +be reported. > + > +If NO-ERROR is nil, signal an error that no VC backend is > +responsible for the given file." > (or (and (not (file-directory-p file)) (vc-backend file)) > (catch 'found > ;; First try: find a responsible backend. If this is for registration, NO-ERROR seems to be a no-op in this patch. Instead of changing the function's arglist, would it be any worse to do the following? (ignore-errors (vc-responsible-backend ...)) Thanks, -- Basil