Revert "Optimize project root detection. "

[MaskRay]

Reverts #470

#470 consisted of 7 commits changing the single place back and forth and that PR got merged with an open issue unaddressed. I don't understand why it got approved and merged eventually. Disregarding the below technical reason, the commits should have been squashed if it was to be merged.

The fallback to default-directory is not a good choice for many language servers as many of them are designed for projects, not arbitrary directories with a single file.

In reality (e.g. when it comes to C/C++ language servers), some files (/usr/include/stdio.h) are in an indeterminate state and don't belong to any specific projects (default-directory /usr/include/ will be bad). The enhancement in workspace folders and session management may help the situation. But before that becomes fully satisfactory, the reasonable approach here is for lsp--suggest-project-root to return nil and just don't start the language server. When those buffers are jumped to from other buffers with associated projects, mark them as "stolen" by those projects.

I understand that in some circumstances the language servers are lenient and the single file is accepted, but please use:

• projectile: (setq projectile-require-project-root nil). Use projectile-ensure-project as mentioned by @seagle0128 .
• project: add your own hackery to project-vc-external-root-functions to allow arbitrary directory to be a project. @seagle0128

Or if the behavior is really reasonable enough, make it a client-specific :get-root setting as @BooAA suggested. This does not make much sense as a lsp-mode default.

The 7 commits made history browsing very difficult. I recommend making a force push to the state before that patch series. @vibhavp @yyoncho

[yyoncho]

I would not take side in this discussion, I just want to mention that force push breaks melpa since it cannot fetch sources.

[seagle0128]

Two cents here.

1. For projectile and project, the behaviors are different if using projectile-ensure-project.
2. Should provide customization or documentation to guide users.

[MaskRay]

@yyoncho Melpa uses commit date. When a new good commit lands, the Melpa version will get updated.

@seagle0128 The owner can make wiki publicly editable so that everyone can contribute the best practice. The behavior difference of projectile and project is unavoidable but you can advice lsp--suggest-project-root or if you do think it makes sense and benefits other lsp-python users, make that change in lsp-python. I don't think this does the good thing in general. You should also have squashed these commits.

[yyoncho]

@MaskRay I did a force push in lsp-java to revert wrongly submitted CL and then melpa didn't fetch the changes for a whole day. I might be wrong, but I believe that melpa just do git pull which fails if there was a force push altering history.

[MaskRay]

https://github.com/melpa/melpa/blob/master/package-build/package-build.el#L282

[yyoncho]

@MaskRay thanks, I guess something else went wrong.

[Riatre]

(I haven't invested a lot of time in Emacs world, apologizes if I say something wrong about Emacs. However I do spent a lot of time in understanding LSP, the following idea is based on how language servers work)

I think the new behavior is going to affect not only C/C++ language servers, but all language servers, except those who does not care about project root at all. So it should be reverted. (Maybe pyls is in this class, I'm not sure)

Notice that the case mentioned by @MaskRay is not limited to C/C++. What would happen if you open /usr/lib/python3/dist-packages/requests/sessions.py first in the editor? In this case you can only hope that the language server does not care about project root, because it is /usr/lib/python3/dist-packages/requests/, and this is certainly wrong for your own Python codes. So (at least before workspace folders arrive) the real assumption made by @seagle0128 's patch is "all language servers shouldn't care about project root and must provide sane behavior even if project root is wrong", otherwise the patch is plainly wrong.

If we do want to make this assumption, we should notify all downstream language server maintainers, or at least put a big warning here, because, it is, in my opinion, pretty insane.

[yyoncho]

@seagle0128 I did some digging related to projectile and adding identity function to projectile root file functions will make it return the current directory in case the other functions fail. Will that work for you?

(setq projectile-project-root-files-functions '(projectile-root-local
                                                projectile-root-bottom-up
                                                projectile-root-top-down
                                                projectile-root-top-down-recurring
                                                identity))

[seagle0128]

@yyoncho it should work. Thanks! I agree to revert as long as all are described clearly in the doc.

BTW, how to handle for project package?

[yyoncho]

@seagle0128

BTW, how to handle for project package?

Can you elaborate?

[seagle0128]

@yyoncho Sure. We need more time to think and discuss.

[yyoncho]

I have reverted the previous change. Let's try to make lsp-mode usable for as much use cases as possible and focus on improving it for everyone. Thanks!