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

👌 Change non-fatal directive parsing errors to warnings #682

Merged
merged 2 commits into from
Jan 12, 2023

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Jan 12, 2023

For non-fatal errors, such as;
faulty options syntax, unknown option keys, and invalid option values,
a warning is raised, but the directive is still run (without the erroneous options).
The warning is given the myst.directive_parse type, which can be suppressed.

For non-fatal errors, such as
faulty options syntax, unknown option keys, and invalid option values,
a warning is raised, but the directive is still run (without the erroneous options).
The warning is given the `myst.directive_parse` type, which can be suppressed.
@codecov
Copy link

codecov bot commented Jan 12, 2023

Codecov Report

Base: 89.84% // Head: 90.17% // Increases project coverage by +0.33% 🎉

Coverage data is based on head (54b6ed2) compared to base (aa3f04d).
Patch coverage: 94.64% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #682      +/-   ##
==========================================
+ Coverage   89.84%   90.17%   +0.33%     
==========================================
  Files          23       23              
  Lines        2560     2586      +26     
==========================================
+ Hits         2300     2332      +32     
+ Misses        260      254       -6     
Flag Coverage Δ
pytests 90.17% <94.64%> (+0.33%) ⬆️

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

Impacted Files Coverage Δ
myst_parser/mocking.py 87.45% <75.00%> (-0.30%) ⬇️
myst_parser/parsers/directives.py 96.36% <95.45%> (+8.58%) ⬆️
myst_parser/mdit_to_docutils/base.py 93.03% <100.00%> (+0.02%) ⬆️
myst_parser/warnings_.py 96.00% <100.00%> (+0.08%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rowanc1
Copy link
Member

rowanc1 commented Jan 12, 2023

Very much want to see this in the JS side as well, it is a bit too strict.

@rowanc1
Copy link
Member

rowanc1 commented Jan 12, 2023

I only looked at the tests, but this seems like a great addition to me 👍. I will follow suit in the JS side soon!

@chrisjsewell
Copy link
Member Author

Very much want to see this in the JS side as well, it is a bit too strict.

yep exactly, I know we talked about this

Either in this PR or a follow, was going to add changing unknown directives and roles to a warning, with a certain warning type

@rowanc1
Copy link
Member

rowanc1 commented Jan 12, 2023

Awesome, can you tag me so I can make the JS one the same!

@chrisjsewell chrisjsewell merged commit ee4c29d into master Jan 12, 2023
@chrisjsewell chrisjsewell deleted the directive-warnings branch January 12, 2023 20:15
@choldgraf
Copy link
Member

was going to add changing unknown directives and roles to a warning, with a certain warning type

+1 from me

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.

3 participants