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

Better comment formatting #87

Merged
merged 6 commits into from
Dec 4, 2020
Merged

Better comment formatting #87

merged 6 commits into from
Dec 4, 2020

Conversation

bricoletc
Copy link
Collaborator

This PR mainly addresses #85, providing better formatting of comments.
It also adds code infrastructure for issuing warnings and ungetting tokens.

Added

  • Infrastructure for issuing warnings in warnings.py. Warnings introduced for comment formatting, as per snakefmt comments parsing has unintended side-effects #85.
  • File-specific logging: warnings and errors during reformatting now automatically refer
    to the raising source file.
  • Infrastructure to 'unget' parsed tokens. Not used yet, but useful to have. With tests.

Fixed

(All these bugs documented in #85)

  • Comments above keywords stay untouched
  • Inline comments in inline-formatted keywords get relocated above keyword
  • PEP8 inline comment formatting: use 2 spaces

With tests for each.

Changed

  • Refactored config-related code out of snakefmt.py and formatter.py to own source
    file, config.py

* Add PEP8 2-space inline comment spacing, with unit test
* Remove leading space in multiple comments in parameters (snakemake#85)
* Add 4 failing test cases taken from snakemake#85
* Add infrastructure to unget tokens from parsed snakefile, with tests
* Parameter values now parse comments occurring before, inline, and
  after them, with mechanism to detect unindented comments based on
  column position
* Centralise functionality in `types.py`, eg PEP8 standards and
  Parameter class logic
* Bugfix: inline comments in inline-formatted keywords get relocated
  above keyword
* Add warning.py for logging warnings
* Add warnings for comments when i)relocated and ii)Appear after code
  they seem to be documenting
* Bugfix: block comments before inline-formatted values get relocated
Place config searching and parsing code to own file, config.py
This de-clutters formatter.py and snakefmt.py
* When traversing source files, modify logger to output the processed
  file name; thus warnings and errors point to the raising file
* De-mix stdout and stderr in CLI tests and refer to each specifically
@bricoletc bricoletc requested a review from mbhall88 as a code owner December 4, 2020 18:03
@codecov
Copy link

codecov bot commented Dec 4, 2020

Codecov Report

Merging #87 (7f79b3e) into dev (9ad270c) will increase coverage by 0.55%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #87      +/-   ##
==========================================
+ Coverage   97.59%   98.14%   +0.55%     
==========================================
  Files           9       11       +2     
  Lines         791      864      +73     
  Branches      146      161      +15     
==========================================
+ Hits          772      848      +76     
+ Misses          9        8       -1     
+ Partials       10        8       -2     
Flag Coverage Δ
unittests 98.14% <100.00%> (+0.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
snakefmt/config.py 100.00% <100.00%> (ø)
snakefmt/formatter.py 99.49% <100.00%> (-0.01%) ⬇️
snakefmt/parser/parser.py 95.91% <100.00%> (+0.21%) ⬆️
snakefmt/parser/syntax.py 99.57% <100.00%> (+1.28%) ⬆️
snakefmt/snakefmt.py 95.97% <100.00%> (-0.33%) ⬇️
snakefmt/types.py 100.00% <100.00%> (ø)
snakefmt/warnings.py 100.00% <100.00%> (ø)

@bricoletc bricoletc merged commit 7f79b3e into snakemake:dev Dec 4, 2020
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.

1 participant