Skip to content

Update Linter for Windows #10

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

Merged
merged 3 commits into from
Sep 28, 2017
Merged

Conversation

deslaughter
Copy link
Contributor

I was having some problems getting the gfortran-based linter to work on Windows 10 so I took a stab at fixing the problems.

Issues:

  1. If no include paths are specified in fortran.includePaths then gfortran to complains about no input files and exits
  2. errorRegex fails to match gfortran output so no errors are displayed

Changes:

  • Modified the getIncludeParams function in src/lib/helper.ts to check for the paths variable having a length of 0. In this case the function returns "" instead of "-I ". The empty argument will then be removed by constructArgumentList in src/features/linter-provider.ts.
  • Modified errorRegex in src/features/linter-provider.ts to allow for upper and lowercase drive names on Windows and to match all space between output lines via \s+ instead of just \n. The change to matching space should allow the regex to work regardless of the operating system using carriage returns or not.

I ran the extension tests on my Windows 10 computer and all 13 passed. Unfortunately, I don't have quick access to a linux system for testing, but I can create a VM if necessary.

The previous revision to errorRegex removed support for \r\n line endings. This change uses \s* to match all whitespace between the lines in the error message which shoul eliminate this problem in the future. Also, the previous version only allowed uppercase drive letters and my system reported lowercase (ie. c:\) which caused the matching to fail.
If no include paths are specified in the configuration, getIncludeParams would return "-I" which would be included in the list of gfortran arguments. This appeared directly before the file path which caused gfortran to complain that no input files were specified. This change allows the include path specification to be removed if there are no include paths specified.
@codecov
Copy link

codecov bot commented Sep 5, 2017

Codecov Report

Merging #10 into master will decrease coverage by 0.12%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
- Coverage    18.9%   18.77%   -0.13%     
==========================================
  Files           9        9              
  Lines         291      293       +2     
  Branches       32       33       +1     
==========================================
  Hits           55       55              
- Misses        236      238       +2
Impacted Files Coverage Δ
src/features/linter-provider.ts 0% <0%> (ø) ⬆️
src/lib/helper.ts 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5169e9...3817c88. Read the comment docs.

The EXIT keyword was added along with the CYCLE keyword to control the program flow within loops. EXIT is equivalent to `break` in C/C++.
@krvajal
Copy link
Collaborator

krvajal commented Sep 28, 2017

This sounds quite good to me.

@krvajal krvajal merged commit dd9757c into fortran-lang:master Sep 28, 2017
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