Fix: abstract limit #268
Merged
Fix: abstract limit #268
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
bug
added 4 commits
April 22, 2024 23:39
What made me start this change were two logic bugs: chars_count being checked against -1 (instead of None) and abstract_delimiter taking higher priority than unlimited chars_count. Apart from fixing the above, this commit reorders the if-elif chain according to priority and, in the documentation, mentions how abstract_chars_count and abstract_delimiter interact. Breaking changes, as far as i'm aware: - abstract_chars_count == -1 is again highest-priority, - more effort (using abstract_delimiter) before non-compliance, - chars_count is now an inclusive setting, increasing the article length before it has to be shortened by one character. None of these changes conflicts with the pre-existing documentation and none should be noticeable in a well-configured environment.
The description of abstract_delimiter was obviously wrong, copy-pasted from abstract_chars_count. Fortunately, even if someone didn't notice this, setting it to a numeric value erred out, therefore nobody can have such a working configuration and there's no need to complicate things trying to accept numbers. The best we can do is document the present behaviour. In order to make the interaction of these two settings clear, describe the full mechanism at once and refer to it from the other place.
One thing not entirely obvious from the changes is that tests/fixtures/docs/page_without_meta_early_delimiter.md together with test_simple_build_item_length_unlimited() checks the priority of abstract_chars_count: -1 versus abstract_delimiter.
Guts
force-pushed
the
fix/abstract-limit
branch
from
2cafc3c to
e78ec88
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #268 +/- ##
==========================================
+ Coverage 84.22% 84.68% +0.45%
==========================================
Files 10 10
Lines 558 555 -3
Branches 119 117 -2
==========================================
Hits 470 470
+ Misses 56 54 -2
+ Partials 32 31 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Co-authored-by: Y.D.X. <[email protected]> Signed-off-by: Julien <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Patches sent by mail by @tiosgz. Applied with
git am. It's my very first time in this kind of git email workflow. Pfiu !.Command used:
Supersedes #202
cc @craigbox @YDX-2147483647