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

Distinguish between classes and elements in pphtml #1242

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

windymilla
Copy link
Collaborator

When checking for unused/undefined CSS, classes and elements were stored in the same array, which was OK for most things. However, if a classname was also an element name (e.g. br) it would be incorrectly reported or ignored.

This commit prepends a period to stored class names to match the class name in the CSS. It also treats anything like <string as a potential element name. This is safer than trying to maintain a list of all valid element names, and if the user has an invalid element name, the validator will tell them, so pphtml doesn't need to do that.

When checking for unused/undefined CSS, classes and elements
were stored in the same array, which was OK for most things.
However, if a classname was also an element name (e.g. `br`)
it would be incorrectly reported or ignored.

This commit prepends a period to stored class names to
match the class name in the CSS. It also treats anything like
`<string` as a potential element name. This is safer than
trying to maintain a list of all valid element names, and if
the user has an invalid element name, the validator will
tell them, so pphtml doesn't need to do that.
@windymilla windymilla requested review from cpeel and srjfoo August 28, 2023 19:27
@windymilla windymilla linked an issue Aug 28, 2023 that may be closed by this pull request
Copy link
Member

@cpeel cpeel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestion on the regex, although it's less a validchar in that case and more a validpattern that would impact how it's used elsewhere.

my $help = 0; # set true if help requested
my $srctext = "book.html"; # default source file
my $outfile = "xxx";
my $validchar = "[-_[:alnum:]]"; # Valid characters for classnames, etc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how precise you care to be here, but valid elements must start with an alpha character, then 0 or more [:alnum:], -, or _:

[:alpha:][-_[:alnum:]]*

rather than

[-_[:alnum:]]+

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't too worried, as it's not really checking valid HTML - just the class use. However, it's probably better to do as you suggest, so it doesn't confuse PPers or future developers, so I'll rustle up another commit - thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpeel - thanks - I've done that now. At least in Perl regex, [:alpha:] can only be used inside a character class, so the regex needed to be [[:alpha:]][-_[:alnum:]]*

Class names have to start with alphabetic character, not number,
hyphen or underscore.
@windymilla windymilla merged commit b952ec8 into DistributedProofreaders:master Sep 7, 2023
1 check passed
@windymilla windymilla deleted the pphtml branch September 7, 2023 19:06
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.

pphtml no longer finds .br when unused
3 participants