jcollard / elm-mode

Elm mode for emacs
GNU General Public License v3.0
373 stars 67 forks source link

project-root definition causes errors with lsp-mode #173

Closed danp closed 3 years ago

danp commented 3 years ago

Hi there,

I opened https://github.com/emacs-lsp/lsp-mode/issues/2335 thinking I was having trouble with lsp-mode. Upon digging, it seems elm-mode's definition of project-root was causing the error I was seeing.

I'm not sure how elm-mode should change, if at all, but opening this for discussion.

cc @dgutov

dgutov commented 3 years ago

@danp Thanks!

Having talked about it more in the linked issue, I'd say elm-mode isn't doing anything criminal (and lsp-mode should take care of the immediate problem on its own end), but you probably still don't want to add a project backend that is predicated on the major mode.

In Elm projects there are all kinds of files, right? It should be weird for a user to see that when an .elm file is open, the current project is this directory, but when they open the README file in the same directory, the current project becomes something else (or is not found at all).

We should rather solve the problem of projects outside of VCS with a better fix for https://debbugs.gnu.org/41572 (which was prematurely closed by Lars).

purcell commented 3 years ago

you probably still don't want to add a project backend that is predicated on the major mode

We don't do that, do we? I feel like I'm misunderstanding something perhaps?

dgutov commented 3 years ago

Oh, sorry.

I jumped to conclusions when I saw a project-find-functions element defined in a major mode package. But you are not adding it anywhere, so it surely doesn't hurt. At least, not by default. And if the user uses APPEND arg to add-hook when adding it, it should avoid the primary drawback.

Still, it shouldn't be necessary. I'll ping you when a better alternative is in place.

purcell commented 3 years ago

Cool, thanks @dgutov

purcell commented 3 years ago

I think we agreed that this wasn't an elm-mode issue particularly, so I'll close this.