-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat] Add Info View buttons: goto file/line/col #13
base: master
Are you sure you want to change the base?
Conversation
This allows one to press <RET> on the link to go to the corresponding file/line/column.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've marked my concerns about the patch in the comments.
(setq lean4-goals goals) | ||
(lean4-info-buffer-redisplay)) | ||
(lean4-info-buffer-redisplay-debounced lsp-buffer-uri)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if threading this state throughout the computation is idiomatic. Perhaps there is a nicer way to retrieve the URI that corresponds to a Goal/PlainGoal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this case different from the lean4-goals/term-goal
defvar
s?
(magit-insert-heading (format "%d:%d: " (1+ (lsp-translate-line line)) (lsp-translate-column character))) | ||
(magit-insert-section (magit-section) | ||
(magit-insert-heading) | ||
(insert-button (format "%s %d:%d" uri (1+ (lsp-translate-line line)) (lsp-translate-column character)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prints out the raw URI, which is like:
file:///home/bollu/work/oss/lean4-metaprogramming-book/lean/extra/attributes.lean 56:0
I imagine there is a way to get the path relative to the LSP root, which I don't know yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The info view will only ever show information from a single file anyway, no? So why include it at all?
(magit-insert-heading (format "%d:%d: " (1+ (lsp-translate-line line)) (lsp-translate-column character))) | ||
(magit-insert-section (magit-section) | ||
(magit-insert-heading) | ||
(insert-button (format "%s %d:%d" uri (1+ (lsp-translate-line line)) (lsp-translate-column character)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should style the button as if it were a magit header. It currently looks like a raw link. I haven't figured this out yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the intended way is to modify the section's keymap, e.g. how RET
in magit goes to whatever is underneath the cursor https://github.com/magit/magit/blob/4e29d5827cb77a7fbcff57033a099e23b8edd424/lisp/magit-status.el#L538-L540
(magit-insert-heading) | ||
(insert-button (format "%s %d:%d" uri (1+ (lsp-translate-line line)) (lsp-translate-column character)) | ||
'action (lambda (btn) | ||
(with-current-buffer (find-file-other-window (lsp--uri-to-path uri)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine emacs
has a nicer API to open the file and go to the correct file, line and column?
(magit-insert-heading (format "%d:%d: " (1+ (lsp-translate-line line)) (lsp-translate-column character))) | ||
(magit-insert-section (magit-section) | ||
(magit-insert-heading) | ||
(insert-button (format "%s %d:%d" uri (1+ (lsp-translate-line line)) (lsp-translate-column character)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The info view will only ever show information from a single file anyway, no? So why include it at all?
(magit-insert-heading (format "%d:%d: " (1+ (lsp-translate-line line)) (lsp-translate-column character))) | ||
(magit-insert-section (magit-section) | ||
(magit-insert-heading) | ||
(insert-button (format "%s %d:%d" uri (1+ (lsp-translate-line line)) (lsp-translate-column character)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the intended way is to modify the section's keymap, e.g. how RET
in magit goes to whatever is underneath the cursor https://github.com/magit/magit/blob/4e29d5827cb77a7fbcff57033a099e23b8edd424/lisp/magit-status.el#L538-L540
(setq lean4-goals goals) | ||
(lean4-info-buffer-redisplay)) | ||
(lean4-info-buffer-redisplay-debounced lsp-buffer-uri)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this case different from the lean4-goals/term-goal
defvar
s?
This allows one to press on the link to go to
the corresponding file/line/column.
Screenshot attached, where the section headers are now clickable links, that take one to the correct file/line/column