-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix couple nits in PPtxt #782
Fix couple nits in PPtxt #782
Conversation
1. Don't report ampersand in character checks because there's a better check later in "Ampersand character in line". 2. Don't report asterisk in ` * * * * *` in character checks - it's a standard thought break.
src/guiguts/tools/pptxt.py
Outdated
# Update dictionary with the weirdos from the line. | ||
for weirdo in weirdos_list: | ||
# Skip exceptions | ||
if weirdo == "*" and line == " *" * 5: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to use a regex, and not assume the # of spaces is always the same. But 5 asterisks should be safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except this way it would warn you if your tb wasn't standard, eg had some spaces missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of variation were you thinking of?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably OK as-is. The spacing between should at least be consistent, but I'm not sure how to specify that in regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that in a regex, if you specify what spacings you want to allow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GG uses 7 spaces on tb conversion, maybe 3 to 10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops - thought break is still reported in character checks if it is centered, which I and others do. 19 leading spaces in my current case, but may vary according to set margins.
Further commit should now permit that. Essentially it will not report if there are 5 equally spaced asterisks which can be indented by any number of spaces greater than or equal to 3 to allow for centering or additional indentation due to blockquotes etc. The spacing between asterisks can be from 3 to 10 spaces.
and the maximum is
Hope that covers all the necessary cases. |
Still reported for centered cases. Did a fresh clone and "gh pr checkout 782" |
Oops - forgot to push my change to Github. Sorry. Should be there now if you refresh your branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK now.
* * * * *
in character checks - it's a standard thought break. Fixes PPtxt: Minor suggestion - do not flag thought breaks #778