KEMBAR78
fix: detect multi-line values if they contain a `=` sign by DDtKey · Pull Request #463 · dotenv-linter/dotenv-linter · GitHub
Skip to content

fix: detect multi-line values if they contain a = sign #463

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 9, 2022
Merged

fix: detect multi-line values if they contain a = sign #463

merged 5 commits into from
Jan 9, 2022

Conversation

DDtKey
Copy link
Member

@DDtKey DDtKey commented Jan 9, 2022

This one is questionable.
I've implemented it in standard way for env libraries and bash, but maybe there is a more suitable approach for the linter 🤔 (As I already mentioned in previous PR's )
It works well, but I mean it makes the QuoteCharacterChecker a little useless.

Closes #460

✔ Checklist:

  • Commit messages have been written in Conventional Commits format;
  • This PR has been added to CHANGELOG.md (at the top of the list);
  • Tests for the changes have been added (for bug fixes / features);
  • Docs have been added / updated on the dotenv-linter.github.io (for bug fixes / features).

@codecov-commenter
Copy link

codecov-commenter commented Jan 9, 2022 •
edited
Loading

Codecov Report

Merging #463 (8ffd028) into master (994b817) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #463      +/-   ##
==========================================
- Coverage   98.88%   98.88%   -0.01%     
==========================================
  Files          41       41              
  Lines        2699     2698       -1     
==========================================
- Hits         2669     2668       -1     
  Misses         30       30
Impacted Files Coverage Δ
src/lib.rs 98.49% <ø> (-0.01%) ⬇️

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 994b817...8ffd028. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jan 9, 2022

Benchmark for 4657c9d

Click to view benchmark
Test PR Benchmark Master Benchmark %
dotenv_linter check 69.0±4.65µs 67.4±4.66µs +2.37%
dotenv_linter compare 49.5±3.08µs 53.1±4.01µs -6.78%
dotenv_linter fix 211.5±103.39µs 212.3±47.12µs -0.38%
dotenv_linter fix with backup 266.0±135.79µs 277.2±89.71µs -4.04%

@mgrachev
Copy link
Member

mgrachev commented Jan 9, 2022

It works well, but I mean it makes the QuoteCharacterChecker a little useless.

Can you give an example of this?

@DDtKey
Copy link
Member Author

DDtKey commented Jan 9, 2022 •
edited
Loading

It works well, but I mean it makes the QuoteCharacterChecker a little useless.

Can you give an example of this?

See changes in complicated.env (63-64) - that's consequences of this changes.
Any non-escaped quote-char in env tells us that this is multi-line start (you can check this right in the shell)

For example, here will be one multi-line value instead of two QuoteCharacter warnings (as it was before)

KEY="VALUE
KEY2=VALUE2
KEY3="VALUE3

The only time we can meet his warning is if there was no closing quote to the end of the file.
For example, in this case we'll receive one QuoteCharacter warning (1 line)

KEY="VALUE
KEY2=VALUE2

In general, it seems to me to be correct 🤔

@DDtKey
Copy link
Member Author

DDtKey commented Jan 9, 2022

The only time we can meet his warning is if there was no closing quote to the end of the file. For example, in this case we'll receive one QuoteCharacter warning (1 line)

I've updated test with this case

@github-actions
Copy link

github-actions bot commented Jan 9, 2022

Benchmark for 02e4dc5

Click to view benchmark
Test PR Benchmark Master Benchmark %
dotenv_linter check 60.4±4.09µs 58.3±3.83µs +3.60%
dotenv_linter compare 45.5±4.05µs 47.4±2.87µs -4.01%
dotenv_linter fix 198.9±104.05µs 192.3±50.24µs +3.43%
dotenv_linter fix with backup 269.6±149.34µs 261.1±132.91µs +3.26%

@github-actions
Copy link

github-actions bot commented Jan 9, 2022

Benchmark for d749de9

Click to view benchmark
Test PR Benchmark Master Benchmark %
dotenv_linter check 59.6±0.35µs 60.2±0.22µs -1.00%
dotenv_linter compare 44.3±0.22µs 44.3±0.17µs 0.00%
dotenv_linter fix 158.6±191.83µs 135.0±76.96µs +17.48%
dotenv_linter fix with backup 179.4±112.72µs 196.6±177.88µs -8.75%

@mgrachev mgrachev merged commit df02210 into master Jan 9, 2022
@mgrachev mgrachev deleted the fix-460 branch January 9, 2022 18:43
@mgrachev
Copy link
Member

mgrachev commented Jan 9, 2022

Thanks a lot 🔥🔥🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgrachev mgrachev mgrachev approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Doesn't detect multi-line values if they contain a = sign

3 participants

@DDtKey @codecov-commenter @mgrachev