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

RFE: Improve rootDirectory detection #826

Closed
har7an opened this issue Nov 26, 2022 · 14 comments · Fixed by #839
Closed

RFE: Improve rootDirectory detection #826

har7an opened this issue Nov 26, 2022 · 14 comments · Fixed by #839
Labels
enhancement New feature or request

Comments

@har7an
Copy link

har7an commented Nov 26, 2022

Hello,

First: Thanks for this amazing project!
Second: I'm using texlab with nvim-lspconfig to build a multi-file project with tectonic. My problem is that I do not want to configure the texlab rootDirectory setting globally in nvim, because I'm working on multiple documents. However, the root directory detection doesn't work correctly otherwise.

I've looked at the code and it seems that when rootDirectory isn't defined (None), it uses a custom fallback in all the necessary locations, which mostly defaults to using the files parent directory. While this allows me to build all files and get diagnostics, it doesn't pick up my custom latexindent config. Would you be interested in an "improved" rootDirectory detection, or is this better left to the editors using LSP?

@pfoerster
Copy link
Member

@har7an
Thanks for the kind words.

Have you tried out #823? It changes a lot of the internals in the project detection method. In particular, rootDirectory is now relative to the main directory instead (e. g. if you have a src folder, it needs to be .. instead). This way, texlab does not rely on the working directory of the editor anymore, which is very prone to errors.

Would you be interested in an "improved" rootDirectory detection

Definitely, which ideas do you have in mind?

In the case of tectonic, we could also parse the Tectonic.toml manifest (there is even a crate for that) and use that to discover the project files.

@har7an
Copy link
Author

har7an commented Nov 26, 2022

Have you tried out #823?

No I haven't, but I can do that of course! Impressive changeset.

Definitely, which ideas do you have in mind?

One could do something similar to nvim: Walk up the file tree and check, in a pre-defined order, for the existence of special files (i.e. .git, Tectonic.toml, ...). Although I guess this brings its own heap of problems in special cases where people e.g. have multiple Tectonic.toml in one git project.

For the time being, I have found this neat fix for nvim users:

require("lspconfig").texlab.setup {
    -- ...
    on_new_config = function(new_config, new_root_dir)
        new_config.settings.texlab.rootDirectory = new_root_dir
    end,
}

@pfoerster
Copy link
Member

One could do something similar to nvim: Walk up the file tree and check, in a pre-defined order, for the existence of special files (i.e. .git, Tectonic.toml, ...)

Yeah, I have thought about this one too. We could extend this list to

  • Tectonic.toml
  • .latexmkrc
  • .chktexrc
  • latexindent config files
  • .git
  • (and maybe even src/source)

For .latexmkrc and .chktexrc we would need a check though to not consider the global config inside the home directory but Tectonic.toml should be pretty safe.

Although I guess this brings its own heap of problems in special cases where people e.g. have multiple Tectonic.toml in one git project

This should be no problem anymore with #823. Root folder detection is now per document instead of globally.

No I haven't, but I can do that of course!

Sure, that would be awesome. I am particulary interested if there are any regressions that the tests do not catch. The performance should be mostly equal but completion should be much snappier.

@har7an
Copy link
Author

har7an commented Nov 26, 2022

#823 definitely improves performance! Unfortunately, it doesn't resolve the rootDirectory issue on its own. It still requires the nvim config I posted above.

Yeah, I have thought about this one too. We could extend this list to

Sounds reasonable, one of those should surely work.

@pfoerster
Copy link
Member

Unfortunately, it doesn't resolve the rootDirectory issue on its own. It still requires the nvim config I posted above.

@har7an Can you give #823 a try again, please? It does now implement the strategy mentioned above.

@pfoerster pfoerster added the enhancement New feature or request label Dec 17, 2022
@xlucn
Copy link

xlucn commented Jan 9, 2023

I am also having issues with the new version 5.0.0.

It seams that .git directory detection has too high priority. For example, I have this git repo structure:

repo/
- .git/
- dir1/
  - main.tex
- dir2/
  - main.tex

Currently, if I edit dir1/main.tex or dir2/main.tex, texlab seems to set root directory to repo/. But I want the root directory to be either dir1 or dir2, depending on which file I am editing. (Update: set rootDirectory = '.' explicitly does not seem to change anything)

So, does the current behavior need improvements? Or, what workaround can I use to achieve what I want? Thanks in advance.

@leon-richardt
Copy link

leon-richardt commented Jan 12, 2023

Transferring a comment from #838 (comment) by @xlucn:


[...]

What I had in mind is more automatic solution. For example, if current file has \begin{document} and \end{document}, then use the folder of current file as root directory. In other words, I hope texlab can find the main document and use its parent folder as root directory. This behavior is intuitive IMHO. (Maybe I should continue the discussion in #826 or in a new issue?)

Here is the reason of the proposal. Texlab is, I guess, executing latexmk from the root directory. So, if the root directory is different from the folder where the main document is, not only the output files are in the wrong place, but texlab also cannot find some included files such as included graphics or tex files. Therefore, I think it's better to determine root directory by the main document, not .git folder or some configuration files.


I think your proposal could work as a "more reasonable" default but I'm not sure it will catch all use cases either. It is possible to have multiple begin{document}/\end{document} pairs in a project, e.g., as in the subfiles example on Overleaf. This is the only example I can think of right now but there might be more.

Still, I think your approach would work very well as a default.1 Maybe it can be combined with #838 for those rare cases where it would fail. On a related note, I'm not sure if implementing a general, always-correct project detection is even possible. TeX is so flexible in the project structures it allows that a general detection must be very hard at least, if not impossible.

Footnotes

  1. Although I don't know how feasible opening and parsing many files would be; this is something the maintainers might be able to shed light on.

@xlucn
Copy link

xlucn commented Jan 12, 2023

@leon-richardt thanks for the reply (in both threads) and transferring the comment.

I looked at vimtex's strategy of dealing with multi file project. Maybe that's helpful for texlab devs, especially the "Directory scan" section. They seems also to cover the subfiles case.

@pfoerster
Copy link
Member

In other words, I hope texlab can find the main document and use its parent folder as root directory.

This was the usual behavior for version 4.x if you set the rootDirectory to null. texlab iteratively checks the parent directory until it finds a suitable parent document. This works relatively fine if you always compile from the same directory as the main file.

However, it does not work if you compile from another directory. Some people like to use a src directory so we needed a way to tell texlab from which directory the document is compiled from. To handle this case, we introduced the rootDirectory setting. In older versions it was based on the current working dir (which is a bad idea since it highly depends on the editor), 5.0.0 makes it relative to the parent document.

Although I don't know how feasible opening and parsing many files would be; this is something the maintainers might be able to shed light on

In my experience, this approach has worked fine in the past (and it still does; salsa might also help us discarding unused parse trees in this case).

Maybe it can be combined with #838 for those rare cases where it would fail. On a related note, I'm not sure if implementing a general, always-correct project detection is even possible.

A simple (and predictable) solution would be the following approach:

For each new/changed document, perform the following steps:

  1. Recursively walk up the directory tree and open the TeX documents on the way up. Stop traversal if we see a texlabroot file or we reach the file system root
  2. For each opened TeX document, we check if it is a parent of the current document. Use the texlabroot directory if available; otherwise use the document directory. If we found a document, then we can stop looking for a parent and resolve the includes of the project.
  3. Load the .aux files (if any) using the texlab.auxDirectory setting by treating the path relative to the root directory. If not set, use the root directory instead.

Further, we would remove special files like .latexmkrc, .chktex or .git directories from the project detection algorithm (maybe with the exception of Tectonic.toml since it splits up document environment over multiple files; we can use the tectonic_docmodel crate for this purpose).

@xlucn @leon-richardt What do you think?

@leon-richardt
Copy link

@xlucn @leon-richardt What do you think?

I don't know how valuable my opinion is as I'm sure you have put much more thought into this than I have. But your suggestion sounds good to me; it would certainly solve my scenario!

Further, we would remove special files like .latexmkrc, .chktex or .git directories from the project detection algorithm (maybe with the exception of Tectonic.toml since it splits up document environment over multiple files; we can use the tectonic_docmodel crate for this purpose).

I believe this would be a wise decision as well so as not to rely on assumptions about a user's project structure 👍

@xlucn
Copy link

xlucn commented Jan 13, 2023

Seems to cover my use case, thanks!

Just one thing, if a non-global .latexmkrc/latexmkrc file exists, doesn't it mean the user wants to execute latexmk in the same directory? That seems to indicate the root directory the user desires (If root directory IS the directory to execute the build command like latexmk, as I understand).

@pfoerster
Copy link
Member

Just one thing, if a non-global .latexmkrc/latexmkrc file exists, doesn't it mean the user wants to execute latexmk in the same directory?

Generally yes, but not always. Consider the .latexmkrc file in the home directory, which can be used to change the global latexmk options. In this case, the file does not necessarily indicate a project.

@leon-richardt @xlucn I have a created a new PR (#839), which implements the algorithm in #826 (comment).

@xlucn
Copy link

xlucn commented Jan 18, 2023

Consider the .latexmkrc file in the home directory

Maybe the "non-global" word is not accurate enough. By that I refer to (using texts in latexmk manpage") "the RC file in the current working directory" except "the system RC file" or "the user's RC file". $HOME/.latexmkrc is "the user's RC file", thus don't use it as project root.

Also, it's similar to (but not entirely) common alg to determine project root based on git repository. The user's home is normally excluded, even if there is a $HOME/.git/ folder.

@har7an
Copy link
Author

har7an commented Jan 19, 2023

Sorry for not responding for so long.

I tested #839 and the project root detection works like a charm now for the project I initially discovered this on. :)

Good job!

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

Successfully merging a pull request may close this issue.

4 participants