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

Update HEEx comment syntax #581

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sodapopcan
Copy link
Collaborator

@sodapopcan sodapopcan commented Sep 12, 2024

This commit prefers the present-day comment syntax for HEEx files.

Looking for a lil' code review here if anyone is up for it otherwise I will merge in a few days.

It's not perfect mainly in that when commenting out a tag like <a> which has its own highlighting, the comment highlighting will be off unless the whole tag is commented out. Similarly, commenting inside <script> and <style> get no special handling. No errors are shown, though, which makes it a nice step up from status-quo.

@jbodah
Copy link
Collaborator

jbodah commented Sep 12, 2024

I haven’t used embedded Elixir enough to be of much help here, however there are testing primitives in this repo for ensuring syntax is correct if you wanted to blast this with tests

@sodapopcan
Copy link
Collaborator Author

Ah right. I have not had much success getting tests running locally, even with Docker. I remember I did a couple of years ago for my first PR recently I just gave up. I will try again. Thanks for the reply!

@jbodah
Copy link
Collaborator

jbodah commented Sep 13, 2024

Which platform are you developing on? I can help you get them running. You'll need (1) gvim or macvim and (2) Ruby to start. Ideally they'd get ported off of Ruby (I can help with that migratoin if we can find a suitable vimscript-based test runner or something)

@sodapopcan
Copy link
Collaborator Author

Thanks for the offer and sorry for the delayed response here. I'm on Mac. I got it working fine a couple of years ago when I made my first PR and wrote a bunch of tests. I'll try it out again this week.

It's great that you bring up getting it off Ruby as I had the same thought. This would effectively kill three dependencies (ruby, docker, and the requirement for a gvim). Unfortunately vim test libs are moving targets. Currently it looks like themis is the only one that is maintained.

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