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

Allow running python -m fortls #197

Merged
merged 2 commits into from
Sep 19, 2022
Merged

Allow running python -m fortls #197

merged 2 commits into from
Sep 19, 2022

Conversation

awvwgk
Copy link
Member

@awvwgk awvwgk commented Sep 19, 2022

Add a __main__.py to allow running python -m fortls.

@awvwgk awvwgk requested a review from gnikit as a code owner September 19, 2022 09:00
@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #197 (dc02c67) into master (8077e2f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #197   +/-   ##
=======================================
  Coverage   86.02%   86.03%           
=======================================
  Files          11       12    +1     
  Lines        4436     4439    +3     
=======================================
+ Hits         3816     3819    +3     
  Misses        620      620           
Impacted Files Coverage Δ
fortls/__main__.py 100.00% <100.00%> (ø)

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

@awvwgk
Copy link
Member Author

awvwgk commented Sep 19, 2022

This pre-commit.ci autofix is quite useful to circumvent the first-time-contributor limitations... not that I plan to do anything malicious, but for running the CI on my patch it is invaluable.

@gnikit
Copy link
Member

gnikit commented Sep 19, 2022

Does adding main.py mean that we should modify the entry_point of fortls?

@awvwgk
Copy link
Member Author

awvwgk commented Sep 19, 2022

Does adding main.py mean that we should modify the entry_point of fortls?

No, they are independent from each other. The entry_point will be linked in $PREFIX/bin/fortls and is only available if $PREFIX/bin is in your PATH, while __main__.py allows running the language server if the fortls module is in the Python path, e.g. when installing in ~/.local and not including ~/.local/bin in your PATH.

@gnikit
Copy link
Member

gnikit commented Sep 19, 2022

Great, thanks for the help. I suppose this has something to do with the brew packaging.

@gnikit gnikit merged commit 229b10f into fortran-lang:master Sep 19, 2022
@awvwgk awvwgk deleted the main-m branch September 19, 2022 10:57
@gnikit
Copy link
Member

gnikit commented Sep 19, 2022

I might have to add back the fortls.py script, in addition to testing it was also used as an entry_point for local testing with vscode i.e. I would set the path to "fortran.fortls.path": ".../fortls/fortls.py" to test certain features in large Fortran projects.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 19, 2022

I would set the path to "fortran.fortls.path": ".../fortls/fortls.py" to test certain features in large Fortran projects.

You should be able to replace this with python -m fortls in the same way I did for the test driver.

@gnikit
Copy link
Member

gnikit commented Sep 19, 2022

I would set the path to "fortran.fortls.path": ".../fortls/fortls.py" to test certain features in large Fortran projects.

You should be able to replace this with python -m fortls in the same way I did for the test driver.

Vscode might (I don't remember) not permit that. Either way it's not very important for me and I have a few changes down the line that would make this a non-issue. As of now, there are solutions like pip install -e . or simply keeping a file like fortls.py somewhere locally.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 23, 2022

As of now, there are solutions like pip install -e . or simply keeping a file like fortls.py somewhere locally.

Editable install seems preferable, if you need to set a path, making __main__.py executable and using fortls/fortls/__main__.py as script should be an option as well.

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