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

Treat Mustache "info" message as info, not problem #274

Merged

Conversation

james-cnz
Copy link
Contributor

The Mustache lint seems to treat all HTML validation messages as problems, even if they are "info".

I have run into this recently, while checking my course format plugin. I guess there's been a recent change to the HTML validator that's caused it? A Mustache file failed validation because some included Moodle Mustache files closed HTML single tags with /> instead of just >. I got the "info" message "Trailing slash on void elements has no effect and interacts badly with unquoted attribute values." As far as I can tell, this message is only ever given when there is no actual problem, and if there is an actual problem with a trailing /, I think another, non-info message will likely be given.

I don't think it's necessary to treat "info" messages as problems, and I suspect it will now cause most course format plugins to fail validation in a way that's beyond the author's control. (Not sure about other plugin types.)

I think the suggested change should fix the issue, but I haven't actually figured out how to run it locally to check, so I don't really know.

I assume this plugin is used for GitHub actions, and the Moodle code prechecks? If so, and this change is accepted, could you tell me how long it's likely to be before those use the update?

@james-cnz
Copy link
Contributor Author

Sorry, I thought I'd run the tests in my fork, but I guess I accidentally ran them against the master branch, rather than my change. Should be right now.

@stronk7
Copy link
Member

stronk7 commented Sep 2, 2023

Hi @james-cnz ,

thanks for the PR and your patience! I started looking to this some days ago, because I had to test other VNU stuff, but had to stop and haven't been able to come back to this till now.

Looking...

@stronk7 stronk7 force-pushed the treat_mustache_info_message_as_info branch from fb74adb to 6d404f0 Compare September 2, 2023 08:50
@stronk7
Copy link
Member

stronk7 commented Sep 2, 2023

Hi again,

I've amended slightly your test (so it continues looking also for errors in the same file), have forced-pushed to your repository.

Once the GHA tests end here (hopefully passing), I'll merge this.

Ciao :-)

@stronk7 stronk7 merged commit 1d76e7d into moodlehq:master Sep 2, 2023
4 checks passed
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.

2 participants