Skip to content
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

Add go-to definition in metadata #4441

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cheerio-pixel
Copy link

Reviving the work of razzmat, I have reworked his pull request to support this feature.

Now if we try to find the definition of a symbol that is defined
externally, it will try to load a stripped-down version of the file where
is defined and show it in a temporal file.
@github-actions github-actions bot added documentation client One or more of lsp-mode language clients labels Apr 26, 2024
@jcs090218
Copy link
Member

@razzmatazz WDYT? 🤔

Copy link
Collaborator

@razzmatazz razzmatazz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with some suggestions to rename omnisharp-related functions to have omnisharp in the name.

(lsp--uri-to-path-1 uri))
)

(defun lsp-csharp--environment-fn ()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest renaming this to lsp-csharp--omnisharp-environment-fn as there are more than 1 LS that are supported in lsp-mode/csharp


file-location))

(defun lsp-csharp--uri->path-fn (uri)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs rename to lsp-csharp--omnisharp-uri->path-fn?

@@ -334,6 +348,61 @@ using the `textDocument/references' request."
(lsp-show-xrefs (lsp--locations-to-xref-items locations-found) nil t)
(message "No references found")))

(defun lsp-csharp--path->qualified-name (path)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lsp-csharp--omnisharp-path->qualified-name?

@@ -41,6 +41,13 @@ Version 1.34.3 minimum is required."
:link '(url-link "https://github.com/OmniSharp/omnisharp-roslyn")
:package-version '(lsp-mode . "9.0.0"))

(defconst lsp-csharp--metadata-uri-re
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lsp-csharp--omnisharp-metadata-uri-re ?

@razzmatazz
Copy link
Collaborator

@razzmatazz WDYT? 🤔

Reviewed, thank you! Approved 👍🏻 , with a suggestion to rename omnisharp-related fns to have omnisharp in the name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client One or more of lsp-mode language clients documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants