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

Use file-truename in lsp-f-canonical #3325

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

Conversation

vspinu
Copy link
Contributor

@vspinu vspinu commented Jan 27, 2022

The problem: symlink any project and access files from the new location. When asked to pick the root press "i". Revert the buffer, it will ask again.

The problem is that `lsp--find-root-interactively returns a true filename of the root(because projectile does) and that is stored in the session file. But then, on subsequent file opening lsp-find-session-folder will not find the root because it operates with sym-linked filename.

I think an ideal scenario would be to differentiate between symlinked and true projects, but given that external tooling returns truenames, I don't think there is a better solution than what is proposed here.

Copy link
Member

@ericdallo ericdallo left a comment

Choose a reason for hiding this comment

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

LGTM

@yyoncho
Copy link
Member

yyoncho commented Jan 28, 2022

The whole point of creating lsp-f- functions was to get rid of the file-truename call because it is dead slow. So we should solve/workaround that issue without using this call.

@kiennq
Copy link
Member

kiennq commented Jan 28, 2022

I agree with @yyoncho, we have been using file-truename before (that's the default behavior of f-canonical...), but it's dead slow and not so intuitive.
For example, if I symlink a folder in another project, I would expect the language server to interpret that folder in the context of the project where it's symlinked instead.

@vspinu
Copy link
Contributor Author

vspinu commented Jan 28, 2022

I agree. I would prefer that everything works in the context as opened in emacs, but one cannot rely on projectile or on project.el backends to return the symliked folder. One can even imagine the situation where sim-link folder is not part of a project but the truename is.

Regarding speed I cannot judge, the part that I am concerned with this PR is for interactive use, so no concern.

@vspinu
Copy link
Contributor Author

vspinu commented Jan 28, 2022

One way to fix this is to alter lsp--suggest-project-root to check if the root is a sub-path of the current dir. If not, chop dir one by one till truename coincides with the found root. When found, return the chopped (symlinked) one, if not the truename root.

If this is ok with you I can alter the PR.

@yyoncho
Copy link
Member

yyoncho commented Jan 28, 2022

Can you give an example of that logic? In general lsp--suggest-project-root is giving only the default value so we can change the way it behaves regarding symlinks with limited/no impact.

@vspinu
Copy link
Contributor Author

vspinu commented Jan 28, 2022

What I meant is if you have R=/home/xyz/project/A/B/file and the project is simlinked such that the path is S=/home/xyz/sym/A/B/file.

When real project root=/home/xyz/project but what we need is /home/xyz/sym as root. What I have now in mind

  1. check if root is a subpath of S, if so S is a real path (no symlinked) - we are good return root
  2. if not, chop S one by one and find root' from the chopped path. Continue till chopped root' is equal to the truename of the chopped path. On each iteration reasign root=root' because it might change in the process.
  3. Return whatever remains from S (in this case home/xyz/sym/)

(2.) can be optimized by chooping first time the entire common tail of the truename and symname.

Why such a complex iterative procedure is required? One could imagine situations where a sub-project of a project A is symlinked as a sub-project of another project B, so the user would expect to get project B as root, but projectile would return project A because it operates on true file paths. Or, as I have seen before, people would simlink parts of the project into a sub-directory within the same project, and they want the sub-directory to be considered as a project.

So it's a fundamental problem with how projectile computes the root. The only backend for project.el project-try-vc does the right thing and returns the symlinked path.

@vspinu
Copy link
Contributor Author

vspinu commented Jan 28, 2022

My proposal from the above is not necessary to fix the current issue. So I am proposing another patch. Also fixing the related diagnostic issue #3326 .

  - expand proj root: project.el backend can return an abbreviated
    root (e.g. ~/path/to/proj)

  - add cached lsp-file-truename
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants