magit / forge

Work with Git forges from the comfort of Magit
GNU General Public License v3.0
1.3k stars 113 forks source link

forge-repository-at-point no longer works #658

Closed ignacio-casso closed 3 months ago

ignacio-casso commented 4 months ago

Hi!

I upgraded all my Emacs packages a few days ago, I'm not sure what Magit version I was using before, but now I'm using 20240415.1557. Since then, forge-repository-at-point and a custom function I had no longer work.

Regarding forge-repository-at-point, I'm not sure how it worked before, but I think it's broken. I expect it to work in any file belonging to a repository or at least in the Magit buffer, but it always tells me that there is no repository at point. So I think it does not work and forge-browse-this-repository does not work either.

I realize this while trying to debug my custom function, which is the one I really care about. It closely related, so maybe the underlying issue is the same. It relies on Magit internals, which I understand that could change at any moment, so I don't expect you to fix anything nor try to debug it (I haven't even tried myself yet). My hope is that fixing the other issue will fix this one automatically or give me some insights about how to fix/adapt my function without me having to invest too much time in it.

I'll describe the issue here anyway, in case you find it useful or know what happens at first glance or want to share any advice about it. The function tries to open a link in Github for the file and line at point, and the code is the following:

(defun ig/forge-browse-url-at-point ()
  (interactive)
  (browse-url
   (concat (forge--format
            (forge-get-repository nil)
            'remote-url-format)
           "/blob/"
           (let
               ((commit-sha (shell-command-to-string "git rev-parse --short HEAD")))
             (if
                 (string-empty-p
                  (shell-command-to-string (concat "git branch -r --contains " commit-sha)))
                 (shell-command-to-string "git rev-parse --short main")
               commit-sha))

           "/"
           (file-relative-name buffer-file-name (projectile-project-root))
           "#L"
           (number-to-string (line-number-at-pos)))))

It fails when trying to get the repository url, with the following trace:

Debugger entered--Lisp error: (cl-no-applicable-method forge--format nil remote-url-format)
  signal(cl-no-applicable-method (forge--format nil remote-url-format))
  cl-no-applicable-method(#s(cl--generic :name forge--format :dispatches ((1 #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>))) (0 #s(cl--generic-generalizer :name eieio--generic-generalizer :priority 50 :tagcode-function cl--generic-struct-tag :specializers-function #f(compiled-function (tag &rest _) #<bytecode 0x438b335fefa5181>)) #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>)))) :method-table (#s(cl--generic-method :specializers (forge-topic t) :qualifiers nil :call-con nil :function #f(compiled-function (topic slot &optional spec) #<bytecode -0xa1f3ddf840f9392>)) #s(cl--generic-method :specializers (forge-post t) :qualifiers nil :call-con nil :function #f(compiled-function (post slot &optional spec) #<bytecode -0x1879eddb81965c66>)) #s(cl--generic-method :specializers (forge-repository t) :qualifiers nil :call-con nil :function #f(compiled-function (repo format-or-slot &optional spec) #<bytecode -0x14f1e98c0f7e3bd4>))) :options nil) nil remote-url-format)
  apply(cl-no-applicable-method #s(cl--generic :name forge--format :dispatches ((1 #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>))) (0 #s(cl--generic-generalizer :name eieio--generic-generalizer :priority 50 :tagcode-function cl--generic-struct-tag :specializers-function #f(compiled-function (tag &rest _) #<bytecode 0x438b335fefa5181>)) #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>)))) :method-table (#s(cl--generic-method :specializers (forge-topic t) :qualifiers nil :call-con nil :function #f(compiled-function (topic slot &optional spec) #<bytecode -0xa1f3ddf840f9392>)) #s(cl--generic-method :specializers (forge-post t) :qualifiers nil :call-con nil :function #f(compiled-function (post slot &optional spec) #<bytecode -0x1879eddb81965c66>)) #s(cl--generic-method :specializers (forge-repository t) :qualifiers nil :call-con nil :function #f(compiled-function (repo format-or-slot &optional spec) #<bytecode -0x14f1e98c0f7e3bd4>))) :options nil) (nil remote-url-format))
  #f(compiled-function (&rest args) #<bytecode 0x1116d39e5d35bbbb>)(nil remote-url-format)
  apply(#f(compiled-function (&rest args) #<bytecode 0x1116d39e5d35bbbb>) nil remote-url-format)
  forge--format(nil remote-url-format)
  (concat (forge--format (forge-get-repository nil) 'remote-url-format) "/blob/" (let ((commit-sha (shell-command-to-string "git rev-parse --short HEAD"))) (if (string-empty-p (shell-command-to-string (concat "git branch -r --contains " commit-sha))) (shell-command-to-string "git rev-parse --short main") commit-sha)) "/" (file-relative-name buffer-file-name (projectile-project-root)) "#L" (number-to-string (line-number-at-pos)))
  (browse-url (concat (forge--format (forge-get-repository nil) 'remote-url-format) "/blob/" (let ((commit-sha (shell-command-to-string "git rev-parse --short HEAD"))) (if (string-empty-p (shell-command-to-string (concat "git branch -r --contains " commit-sha))) (shell-command-to-string "git rev-parse --short main") commit-sha)) "/" (file-relative-name buffer-file-name (projectile-project-root)) "#L" (number-to-string (line-number-at-pos))))
  ig/forge-browse-url-at-point()
  funcall-interactively(ig/forge-browse-url-at-point)
  call-interactively(ig/forge-browse-url-at-point record nil)
  command-execute(ig/forge-browse-url-at-point record)
  counsel-M-x-action("ig/forge-browse-url-at-point")
  #f(compiled-function (x) #<bytecode -0x18a0a7a57c44c540>)("ig/forge-browse-url-at-point")
  ivy-call()
  #f(compiled-function (arg1 arg2 &rest rest) "Read a string in the minibuffer, with completion.\n\nPROMPT is a string, normally ending in a colon and a space.\n`ivy-count-format' is prepended to PROMPT during completion.\n\nCOLLECTION is either a list of strings, a function, an alist, or\na hash table, supplied for `minibuffer-completion-table'.\n\nPREDICATE is applied to filter out the COLLECTION immediately.\nThis argument is for compatibility with `completing-read'.\n\nWhen REQUIRE-MATCH is non-nil, only members of COLLECTION can be\nselected. In can also be a lambda.\n\nIf INITIAL-INPUT is non-nil, then insert that input in the\nminibuffer initially.\n\nHISTORY is a name of a variable to hold the completion session\nhistory.\n\nKEYMAP is composed with `ivy-minibuffer-map'.\n\nPRESELECT, when non-nil, determines which one of the candidates\nmatching INITIAL-INPUT to select initially.  An integer stands\nfor the position of the desired candidate in the collection,\ncounting from zero.  Otherwise, use the first occurrence of\nPRESELECT in the collection.  Comparison is first done with\n`equal'.  If that fails, and when applicable, match PRESELECT as\na regular expression.\n\nDEF is for compatibility with `completing-read'.\n\nUPDATE-FN is called each time the candidate list is re-displayed.\n\nWhen SORT is non-nil, `ivy-sort-functions-alist' determines how\nto sort candidates before displaying them.\n\nACTION is a function to call after selecting a candidate.\nIt takes one argument, the selected candidate. If COLLECTION is\nan alist, the argument is a cons cell, otherwise it's a string.\n\nMULTI-ACTION, when non-nil, is called instead of ACTION when\nthere are marked candidates. It takes the list of candidates as\nits only argument. When it's nil, ACTION is called on each marked\ncandidate.\n\nUNWIND is a function of no arguments to call before exiting.\n\nRE-BUILDER is a function transforming input text into a regex\npattern.\n\nMATCHER is a function which can override how candidates are\nfiltered based on user input.  It takes a regex pattern and a\nlist of candidates, and returns the list of matching candidates.\n\nDYNAMIC-COLLECTION is a boolean specifying whether the list of\ncandidates is updated after each input by calling COLLECTION.\n\nEXTRA-PROPS is a plist that can be used to store\ncollection-specific session-specific data.\n\nCALLER is a symbol to uniquely identify the caller to `ivy-read'.\nIt is used, along with COLLECTION, to determine which\ncustomizations apply to the current completion session." #<bytecode -0x345c58e22c6407e>)("M-x " [org-capture-goto-last-stored magit-section-show-headings vc-src-responsible-p soap-xs-complex-type-is-group--cmacro tramp-sudoedit-file-name-handler tramp-completion-handle-file-name-all-completions projectile-get-project-directories telega-msg-open-thread-or-topic telega-company-tooltip-always-below vc-sccs-log-view-mode-hook telega-chats-filtered-toggle-read magit-revision-filter-files-on-follow treepy-vector-zip yas-x-prompt gnus-article-nndoc-name dired-unmark-all-marks flycheck-mode-line-prefix magit-xref-insert-button gnus-mime-security-details-buffer gnus-agent-group-covered-p tramp-compat-string-equal-ignore-case telega-ewoc--gen-pp game transient:magit-diff-refresh:--irreversible-delete treemacs--button-symbol-switch yaml-pro-format-ts-indent-groups updateFile magit-list-modified-modules pullreq_label:pullreq org-switch-to-buffer-other-window EXPLAIN flycheck-pug-executable tramp-sudoedit-handle-set-file-acl org-table-get-range vc-bzr-shelve-menu About ov-end swiper-include-line-number-in-search gnus-summary-limit-to-marks org-agenda-menu-show-matcher telega-photo--progress-svg :story_stealth_mode_cooldown_period filename-and-process projectile-unserialize star_count jiralib-fields-for-action-cache-p Noto\ Sans\ Gurmukhi counsel--async-filter tramp-handle-make-auto-save-file-name zot ...] :predicate #f(compiled-function (sym) #<bytecode 0x40f952112e583c4>) :require-match t :history counsel-M-x-history :action counsel-M-x-action :keymap (keymap (67108908 . counsel--info-lookup-symbol) (67108910 . counsel-find-symbol)) :initial-input nil :caller counsel-M-x :sort t)
  apply(#f(compiled-function (arg1 arg2 &rest rest) "Read a string in the minibuffer, with completion.\n\nPROMPT is a string, normally ending in a colon and a space.\n`ivy-count-format' is prepended to PROMPT during completion.\n\nCOLLECTION is either a list of strings, a function, an alist, or\na hash table, supplied for `minibuffer-completion-table'.\n\nPREDICATE is applied to filter out the COLLECTION immediately.\nThis argument is for compatibility with `completing-read'.\n\nWhen REQUIRE-MATCH is non-nil, only members of COLLECTION can be\nselected. In can also be a lambda.\n\nIf INITIAL-INPUT is non-nil, then insert that input in the\nminibuffer initially.\n\nHISTORY is a name of a variable to hold the completion session\nhistory.\n\nKEYMAP is composed with `ivy-minibuffer-map'.\n\nPRESELECT, when non-nil, determines which one of the candidates\nmatching INITIAL-INPUT to select initially.  An integer stands\nfor the position of the desired candidate in the collection,\ncounting from zero.  Otherwise, use the first occurrence of\nPRESELECT in the collection.  Comparison is first done with\n`equal'.  If that fails, and when applicable, match PRESELECT as\na regular expression.\n\nDEF is for compatibility with `completing-read'.\n\nUPDATE-FN is called each time the candidate list is re-displayed.\n\nWhen SORT is non-nil, `ivy-sort-functions-alist' determines how\nto sort candidates before displaying them.\n\nACTION is a function to call after selecting a candidate.\nIt takes one argument, the selected candidate. If COLLECTION is\nan alist, the argument is a cons cell, otherwise it's a string.\n\nMULTI-ACTION, when non-nil, is called instead of ACTION when\nthere are marked candidates. It takes the list of candidates as\nits only argument. When it's nil, ACTION is called on each marked\ncandidate.\n\nUNWIND is a function of no arguments to call before exiting.\n\nRE-BUILDER is a function transforming input text into a regex\npattern.\n\nMATCHER is a function which can override how candidates are\nfiltered based on user input.  It takes a regex pattern and a\nlist of candidates, and returns the list of matching candidates.\n\nDYNAMIC-COLLECTION is a boolean specifying whether the list of\ncandidates is updated after each input by calling COLLECTION.\n\nEXTRA-PROPS is a plist that can be used to store\ncollection-specific session-specific data.\n\nCALLER is a symbol to uniquely identify the caller to `ivy-read'.\nIt is used, along with COLLECTION, to determine which\ncustomizations apply to the current completion session." #<bytecode -0x345c58e22c6407e>) ("M-x " [org-capture-goto-last-stored magit-section-show-headings vc-src-responsible-p soap-xs-complex-type-is-group--cmacro tramp-sudoedit-file-name-handler tramp-completion-handle-file-name-all-completions projectile-get-project-directories telega-msg-open-thread-or-topic telega-company-tooltip-always-below vc-sccs-log-view-mode-hook telega-chats-filtered-toggle-read magit-revision-filter-files-on-follow treepy-vector-zip yas-x-prompt gnus-article-nndoc-name dired-unmark-all-marks flycheck-mode-line-prefix magit-xref-insert-button gnus-mime-security-details-buffer gnus-agent-group-covered-p tramp-compat-string-equal-ignore-case telega-ewoc--gen-pp game transient:magit-diff-refresh:--irreversible-delete treemacs--button-symbol-switch yaml-pro-format-ts-indent-groups updateFile magit-list-modified-modules pullreq_label:pullreq org-switch-to-buffer-other-window EXPLAIN flycheck-pug-executable tramp-sudoedit-handle-set-file-acl org-table-get-range vc-bzr-shelve-menu About ov-end swiper-include-line-number-in-search gnus-summary-limit-to-marks org-agenda-menu-show-matcher telega-photo--progress-svg :story_stealth_mode_cooldown_period filename-and-process projectile-unserialize star_count jiralib-fields-for-action-cache-p Noto\ Sans\ Gurmukhi counsel--async-filter tramp-handle-make-auto-save-file-name zot ...] :predicate #f(compiled-function (sym) #<bytecode 0x40f952112e583c4>) :require-match t :history counsel-M-x-history :action counsel-M-x-action :keymap (keymap (67108908 . counsel--info-lookup-symbol) (67108910 . counsel-find-symbol)) :initial-input nil :caller counsel-M-x :sort t))
  ivy-read("M-x " [org-capture-goto-last-stored magit-section-show-headings vc-src-responsible-p soap-xs-complex-type-is-group--cmacro tramp-sudoedit-file-name-handler tramp-completion-handle-file-name-all-completions projectile-get-project-directories telega-msg-open-thread-or-topic telega-company-tooltip-always-below vc-sccs-log-view-mode-hook telega-chats-filtered-toggle-read magit-revision-filter-files-on-follow treepy-vector-zip yas-x-prompt gnus-article-nndoc-name dired-unmark-all-marks flycheck-mode-line-prefix magit-xref-insert-button gnus-mime-security-details-buffer gnus-agent-group-covered-p tramp-compat-string-equal-ignore-case telega-ewoc--gen-pp game transient:magit-diff-refresh:--irreversible-delete treemacs--button-symbol-switch yaml-pro-format-ts-indent-groups updateFile magit-list-modified-modules pullreq_label:pullreq org-switch-to-buffer-other-window EXPLAIN flycheck-pug-executable tramp-sudoedit-handle-set-file-acl org-table-get-range vc-bzr-shelve-menu About ov-end swiper-include-line-number-in-search gnus-summary-limit-to-marks org-agenda-menu-show-matcher telega-photo--progress-svg :story_stealth_mode_cooldown_period filename-and-process projectile-unserialize star_count jiralib-fields-for-action-cache-p Noto\ Sans\ Gurmukhi counsel--async-filter tramp-handle-make-auto-save-file-name zot ...] :predicate #f(compiled-function (sym) #<bytecode 0x40f952112e583c4>) :require-match t :history counsel-M-x-history :action counsel-M-x-action :keymap (keymap (67108908 . counsel--info-lookup-symbol) (67108910 . counsel-find-symbol)) :initial-input nil :caller counsel-M-x)
  counsel-M-x()
  funcall-interactively(counsel-M-x)
  call-interactively(counsel-M-x nil nil)
  command-execute(counsel-M-x)

Thanks!

tarsius commented 4 months ago

Like many other parts of Forge, this area also received some heavy refactoring over the last few months. I've concentrated on paying of technical dept in core abstractions and the fine-tuning on top of that isn't quite done yet. (Over the next two weeks I will finish the last core refactoring and then I plan to do about two weeks of fine-tuning, before I create a release.)

Regarding forge-repository-at-point, I'm not sure how it worked before, but I think it's broken. I expect it to work in any file belonging to a repository or at least in the Magit buffer, but it always tells me that there is no repository at point. So I think it does not work and forge-browse-this-repository does not work either.

As the name suggests, this returns the repository at point. There is no repository at point in a file visiting buffer, so here that returns nil. (Well okay, there may be a file-name at point, which could be resolved to a repository, but this function ignores that, as it is only intended to work in magit-mode derived buffers, and some tabulated-list-mode derived buffers.)

The two functions you might want to use instead are forge-current-repository and forge-get-repository, the workhorse.

Since the fine-tuning is still two weeks of, here's what you probably need:

(browse-url (forge-get-url (forge-get-repository :stub)))

forge-browse-this-repository should probably behave like that (except it should also use forge-repository-at-point), but I'll have to think about that some more in two weeks (e.g., should forge-current-repository also get a demand argument?).

I recommend you read the short implementations of forge-current-repository and forge-repository-at-point, and the docstring of forge-get-repository.

ignacio-casso commented 4 months ago

Thanks for your advice! I will try with that for my function.

Regarding forge-repository-at-point:

I expect it to work in any file belonging to a repository or at least in the Magit buffer, but it always tells me that there is no repository at point.

it is only intended to work in magit-mode derived buffers

Well then it should work in the Magit status buffer right? Because I tried using forge-browse-this-repository there and it didn't work, it also says "No repository at point". Anyway, I always use forge-browse-repository which still works fine, it was just to let you know. Thanks!

tarsius commented 4 months ago

it is only intended to work in magit-mode derived buffers

Well then it should work in the Magit status buffer right?

There operative term is at point. The fact that it only works in magit buffers is an additional restriction, which could, and might be, lifted. But this function and other forge-...-at-point functions will always just return the thing at point, that's their purpose. The forge-current-... functions were created to handle the "the thing at point, or if there is no thing at point, then fall back to the thing the buffer as a whole is about" case.