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

mention eglot for emacs support #129

Merged
merged 1 commit into from
Aug 21, 2022
Merged

Conversation

ssnnoo
Copy link
Contributor

@ssnnoo ssnnoo commented May 24, 2022

eglot is a popular lsp client for emacs that supports fortls.

@ssnnoo ssnnoo requested a review from gnikit as a code owner May 24, 2022 20:44
@codecov
Copy link

codecov bot commented May 25, 2022

Codecov Report

Merging #129 (447c6af) into master (34f5bbf) will not change coverage.
The diff coverage is n/a.

❗ Current head 447c6af differs from pull request most recent head 6053c22. Consider uploading reports for the commit 6053c22 to get more accurate results

@@           Coverage Diff           @@
##           master     #129   +/-   ##
=======================================
  Coverage   86.00%   86.00%           
=======================================
  Files          11       11           
  Lines        4430     4430           
=======================================
  Hits         3810     3810           
  Misses        620      620           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@gnikit gnikit left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, a few things:

  1. Multiple plugins should preferably be in their own subsection see the Vim section (the Atom section is not a good example since my extension it is a WIP)
  2. All the necessary instructions on how to configure fortls with eglot should be provided so users can copy-paste them in their configs. It is important to set reasonable options in the config.
  3. I don't know how eglot works or if it downloads the server for you but their README link to fortls is wrong, it is pointing to the fortran-language-server project not this repo.

@ssnnoo
Copy link
Contributor Author

ssnnoo commented May 25, 2022

Ok, I will update the pull request tonight.

Regarding the wrong link please see joaotavora/eglot#965, hopefully this will be updated soon.

Last but not least: if in f90 mode, eglot will start fortls as default, so no copy and paste required :-)

@ssnnoo
Copy link
Contributor Author

ssnnoo commented May 31, 2022

Ok, I updated the pull request.

Unfortunately, nothing has happend regarding joaotavora/eglot#965 yet.

@gnikit
Copy link
Member

gnikit commented Jul 12, 2022

Any news on this @ssnnoo ?

@ssnnoo
Copy link
Contributor Author

ssnnoo commented Jul 12, 2022

Any news on this @ssnnoo ?

Hi, I changed the commit as requested a while ago and it should be fine now (hopefully). One thing that did not happen is that joaotavora/eglot#965 got merged. But this is completely independent of this contribution here :-)

@gnikit
Copy link
Member

gnikit commented Jul 12, 2022

Any news on this @ssnnoo ?

Hi, I changed the commit as requested a while ago and it should be fine now (hopefully). One thing that did not happen is that joaotavora/eglot#965 got merged. But this is completely independent of this contribution here :-)

The problem is that the underlying extension is pointing to the wrong server, which leads to confusion from the user's perspective. I will go drop a comment

@ssnnoo
Copy link
Contributor Author

ssnnoo commented Jul 12, 2022

Any news on this @ssnnoo ?

Hi, I changed the commit as requested a while ago and it should be fine now (hopefully). One thing that did not happen is that joaotavora/eglot#965 got merged. But this is completely independent of this contribution here :-)

The problem is that the underlying extension is pointing to the wrong server, which leads to confusion from the user's perspective. I will go drop a comment

Sure, I agree. But this is only in the readme, in the source code it only refers to fortls.

((fortran-mode f90-mode) . ("fortls")) and this should be fine. There is not automatic installation or anything.

Copy link
Member

@gnikit gnikit left a comment

Choose a reason for hiding this comment

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

Great work @ssnnoo

@gnikit gnikit merged commit b615126 into fortran-lang:master Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants