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

[feature request] change (as opposed to rename) environement #849

Closed
martinra opened this issue Feb 7, 2023 · 7 comments
Closed

[feature request] change (as opposed to rename) environement #849

martinra opened this issue Feb 7, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@martinra
Copy link
Contributor

martinra commented Feb 7, 2023

I would like to add a feature to change an environment, which when triggered changes ENV in \begin{ENV} and \end{ENV} of the innermost environment relative to the cursor. This would recover a feature of vimtex.

I think I can implement the set of code changes paralleling the implementations in features/rename. However, when trying to plan an implementation and how to expose the feature, I first looked into rename and found that its semantics are not the one I am looking for. Looking further through the LSP documentation, I did not find any CodeAction that encodes it.

Is there room for such a feature in texlab? And if so, what part of the LSP should one use to expose it?

@pfoerster
Copy link
Member

I would like to add a feature to change an environment, which when triggered changes ENV in \begin{ENV} and \end{ENV} of the innermost environment relative to the cursor.

I am bit surprised to see that this feature is no longer there since version 3.0.0 and I am not sure why. This was definitely an accidental change. See here: https://github.com/latex-lsp/texlab/blob/v2.2.2/src/rename/latex_env.rs

I first looked into rename and found that its semantics are not the one I am looking for.

I agree that textDocument/rename is not the correct request for this feature when we re-introduce this feature. In this case, the user likely would expect that all occurrences are renamed too.

And if so, what part of the LSP should one use to expose it?

I think CodeActions are fine for this feature.

I did not find any CodeAction that encodes it.

Actually, we are free to define our own code action (e. g. something like 'source.changeEnvironment'). Looking at the type definition, we can see that the kind is just a string. In fact, the server must announce the supported actions during initialization.

Is there room for such a feature in texlab?

Sure :)

@pfoerster pfoerster added the enhancement New feature or request label Feb 7, 2023
@pfoerster
Copy link
Member

@martinra

I think CodeActions are fine for this feature.

Actually, I have to correct myself here. Unfortunately, code actions do not allow passing additional parameters which is needed in this case to set the new name. I think, we have to resort to the textDocument/rename request although it has strictly speaking not the correct semantics.

@martinra
Copy link
Contributor Author

I have thought about possibilities to avoid locking texlab out of the possibility to rename environments. textDocument/rename with the position on an environment would ideally rename that environment as that's what the LSP documentation suggests. So far I could think of two possibilities:

(1) One possibility is to use textDocument/rename on a \begin or \end part of \begin{ENV} ... \end{ENV}. This would strictly speaking rename the keyword begin or end, which is anyway not allowed. This way we would occupy a somewhat empty niche in the semantic space.

(2) I read through the exact documentation of CodeAction and found one way to pass parameters without completely violating the protocol. There is a diagnostics argument, that is intended to support the server in finding suitable CodeActions. One could pass something along the lines of "Can be renamed to ENVNEW". The server is allows to ignore the diagnostics argument, so this would be forward/backward compatible.

Right now I think (1) is a quite suitable way of doing things. Can you give me some critical feedback on this?

If we go for either (or any third solution), I think I'll write the rename environment and change environment in one merge request, if that suits you. This would be to avoid any oversights in their interaction when I wrap things in neovim.

@pfoerster
Copy link
Member

(1) One possibility is to use textDocument/rename on a \begin or \end part of \begin{ENV} ... \end{ENV}. This would strictly speaking rename the keyword begin or end, which is anyway not allowed. This way we would occupy a somewhat empty niche in the semantic space.

While this would technically work, it kind of messes with the textDocument/prepareRename request. This request would either need to return the range of the \begin (which could be confusing) or move the rename range outside of the cursor (which is not explicity disallowed but feels wrong to do).

(2) I read through the exact documentation of CodeAction and found one way to pass parameters without completely violating the protocol. There is a diagnostics argument, that is intended to support the server in finding suitable CodeActions. One could pass something along the lines of "Can be renamed to ENVNEW"

I am still not sure how the server would receive the new name. The code action needs to contain a Command or a WorkspaceEdit, which we cannot provide because we do not know the new name.

Speaking about Command: I think, the only spec-compliant way to (re-)implement this feature right now would be to provide a custom command (something like texlab.changeEnvironment via workspace/executeCommand) that receives the cursor position and the new name. The command then applies a WorkspaceEdit which renames the environment. In this case, the client would be responsible for calling the command with the required arguments (e. g. command palette in VSCode or a custom Vim command). Maybe, we can optionally provide a command to allow the client to check if there is actually an environment at the cursor position.

@clason
Copy link
Contributor

clason commented Feb 12, 2023

Yes, a custom command is the way to go here. Texlab has always been examplary at implementing the specification exactly (not like some other servers I could mention...)

And just as a reminder from the Neovim side: it was always the official design goal of the built-in LSP client to have server-specific plugins that handle custom commands and whatnot. Lspconfig was only ever meant to get the "simple" server up and running quickly, without bells and whistles.

@martinra
Copy link
Contributor Author

Thank you! I wasn't aware of workspace/executeCommand (still lerning LSP). I'm at it and learning more of the texlab architecture. It might be a bit slow, but I'll post once I have an implementation.

@pfoerster
Copy link
Member

@martinra Thank you for the PR! This feature has been released with v5.4.0 🚀

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Apr 27, 2023
## [5.5.0] - 2023-04-16

### Added

- Allow optionally passing cursor position to `textDocument/build` request for use in forward search after building.
  Previously, the server had to guess the cursor position ([#475](latex-lsp/texlab#475))
- Add experimental `texlab.experimental.citationCommands` setting to allow extending the list of citation commands
  ([#832](latex-lsp/texlab#832))
- Add support for escaping placeholders in build arguments similar to forward search
- Allow configuring completion matching algorithm ([#872](latex-lsp/texlab#872))

### Fixed

- Fix regression introduced in `v5.4.2` involving `texlab.cleanArtifacts` command.

## [5.4.2] - 2023-04-11

### Fixed

- Fix memory leak when editing documents over a long time ([#856](latex-lsp/texlab#856))
- Fix parsing parentheses in file paths ([#874](latex-lsp/texlab#874))

## [5.4.1] - 2023-03-26

### Fixed

- Do not return symbols with empty names (e. g. sections without name) ([#870](latex-lsp/texlab#870))
- Repair `textDocument/formatting` request ([#871](latex-lsp/texlab#871))

## [5.4.0] - 2023-03-12

### Added

- Add experimental settings to allow extending the list of special environments:
  - `texlab.experimental.mathEnvironments`
  - `texlab.experimental.enumEnvironments`
  - `texlab.experimental.verbatimEnvironments`
- Add `texlab.changeEnvironment` workspace command ([#849](latex-lsp/texlab#849))
- Add `texlab.showDependencyGraph` workspace command

### Changed

- Do not show caption or section names in label inlay hints ([#858](latex-lsp/texlab#858))
- Include more user-defined commands in command completion

### Fixed

- Parse nested `\iffalse` blocks correctly ([#853](latex-lsp/texlab#853))
- Parse commands with multi-byte characters correctly ([#857](latex-lsp/texlab#857))
- Fix checking whether a document can be a root file

## [5.3.0] - 2023-02-25

### Added

- Allow filtering `textDocument/documentSymbols` using regular expressions specified via
  `texlab.symbols.allowedPatterns` and `texlab.symbols.ignoredPatterns`
  ([#851](latex-lsp/texlab#851))

### Fixed

- Do not use percent-encoded path when searching for PDF files during forward search
  ([#848](latex-lsp/texlab#848))
- Always return an empty list of code actions instead of returning "method not found" ([#850](latex-lsp/texlab#850))

## [5.2.0] - 2023-01-29

### Added

- Include line numbers in build warnings when available ([#840](latex-lsp/texlab#840))
- Add `none` formatter to `texlab.latexFormatter` and `texlab.bibtexFormatter` options
  to allow disabling formatting ([#846](latex-lsp/texlab#846))

### Fixed

- Concatenate more than two lines of maximum length in build diagnostics ([#842](latex-lsp/texlab#842))
- Apply the correct range of references to labels when renaming ([#841](latex-lsp/texlab#841))
- Use `document` environment to detect root file instead of `\documentclass` ([#845](latex-lsp/texlab#845))

## [5.1.0] - 2023-01-21

### Added

- Allow manually overriding the root directory using a `texlabroot`/`.texlabroot` marker file.
  See the wiki for more information.
  ([#826](latex-lsp/texlab#826), [#838](latex-lsp/texlab#838))

### Deprecated

- Deprecate `texlab.rootDirectory` setting in favor of `.texlabroot` files

### Fixed

- Do not use `.git`, `.chktexrc`, `.latexmkrc` files/directories to determine the root directory
  ([#826](latex-lsp/texlab#826))
- Fix building documents without an explicit root directory ([#837](latex-lsp/texlab#837))

## [5.0.0] - 2022-12-29

### Changed

- _BREAKING_: `texlab.rootDirectory` is now used as the folder path from which the compiler is executed
  relative to the main document. By default it is equal to `"."`. For more information, please visit the wiki.
- Improve performance of completion by a huge margin due to a faster filtering method used internally
- Do not discover project files beyond the provided workspace folders
- Try to guess the root directory by checking for files such as `.latexmkrc` or `Tectonic.toml` if `texlab.rootDirectory` is not set

### Fixed

- Update positions of reported build diagnostics when editing the affected line
- Do not treat links to files as bidirectional by default. This prevents issues where `texlab` ends up compiling the wrong file
  in projects with shared files ([#806](latex-lsp/texlab#806), [#757](latex-lsp/texlab#757), [#679](latex-lsp/texlab#679))
- Fix coverage of directories which need to be watched for changes ([#502](latex-lsp/texlab#502), [#491](latex-lsp/texlab#491))
- Resolve links of the `import` package correctly
- Use `filterText` of completion items when filtering internally ([#829](latex-lsp/texlab#829))

## [4.3.2] - 2022-11-20

### Fixed

- Do not try to run the TeX engine on package files and fail the build instead ([#801](latex-lsp/texlab#801))
- Handle URIs with URL-encoded drive letters on Windows ([#802](latex-lsp/texlab#802))
- Parse BibTeX entries with unbalanced quotes correctly ([#809](latex-lsp/texlab#809))
- Provide completion for more acronym commands ([#813](latex-lsp/texlab#813))
- Fix parsing acronym definitions ([#813](latex-lsp/texlab#813))

## [4.3.1] - 2022-10-22

### Fixed

- Do not crash with a stack overflow when trying to load packages with many internal dependencies ([#793](latex-lsp/texlab#793))
- Normalize drive letters of all document URIs
- Fix parsing commands that take file paths as arguments ([#789](latex-lsp/texlab#789))
- Use the correct working directory and command line arguments when calling `latexindent` ([#645](latex-lsp/texlab#645))
- Fix publishing to CTAN

## [4.3.0] - 2022-09-25

### Added

- Add inlay hints for `\label{...}` ([#753](latex-lsp/texlab#753))

### Fixed

- Improve accuracy of the error locations reported by the TeX engine ([#738](latex-lsp/texlab#738))
- Reduce number of false positive errors reported by `texlab` ([#745](latex-lsp/texlab#745))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants