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

Some minor changes to make LSP use easier #21

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Conversation

dgkf
Copy link
Contributor

@dgkf dgkf commented Aug 25, 2024

  • Allows for passing pre-calculated xml parse data as part of the reshape_info(info = ) parameter, since this is already pre-parsed by the lsp.
  • Exports reshape_info

The reshape_info() interface is just about perfect for the LSP as it is. You can see how it's used in this function in my fork of {languageserver} in these lines. However, the RStudio addin and emacs interfaces have more bespoke wrappers. Since the rest of the logic is lsp-implementation-specific, it seemed most natural to just export reshape_info, but I would understand if you'd prefer to keep a more consistent with the other tool interfaces.

@lionel- lionel- merged commit 1d4a809 into lionel-:main Aug 29, 2024
@lionel-
Copy link
Owner

lionel- commented Aug 29, 2024

I don't mind exporting reshape_info() but I don't see it exported here? I'll merge this work already, feel free to send a new PR to export and document. I guess this means parse_info() needs to be exported as well.

I noticed a typo in conditioNMessage in your languageserver changelist:

            logger$info(
                "R Error encountered while preparing {codegrip} reshape code action: ",
                conditioNMessage(e)
            )

Thanks for working on codegrip!

@dgkf
Copy link
Contributor Author

dgkf commented Aug 29, 2024

🤦 Thanks for catching that. Was juggling too many branches. Will submit a new PR to export it shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants