Skip to content

js-beautify support for mustache and handlebars #170

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

Merged
merged 1 commit into from
Mar 28, 2021
Merged

js-beautify support for mustache and handlebars #170

merged 1 commit into from
Mar 28, 2021

Conversation

abgeana
Copy link

@abgeana abgeana commented Mar 27, 2021

The mustache/vim-mustache-handlebars plugin sets the filetype to either
html.handlebars or html.mustache. Even though js-beautify supports
formatting for these files, the --type parameter must be html.

This commit sets the --type parameter to html when the filetype
matches the regex html\..*.

@google-cla
Copy link

google-cla bot commented Mar 27, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Mar 27, 2021
@abgeana
Copy link
Author

abgeana commented Mar 27, 2021

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Mar 27, 2021
@abgeana
Copy link
Author

abgeana commented Mar 27, 2021

CI is still failing, but I see in the tests for js-beautify are fine this time. The problem is with another formatter I think.

@dbarnett
Copy link
Contributor

dbarnett commented Mar 27, 2021

CI is still failing, but I see in the tests for js-beautify are fine this time. The problem is with another formatter I think.

Interesting, looks like #167 and for some reason Travis never ran on that. I'll look into it.

EDIT: I have PR #171 to fix the test issues.

Comment on lines 33 to 35
return &filetype is# 'css' || &filetype is# 'html' || &filetype is# 'json' ||
return &filetype is# 'css' || &filetype =~ 'html.*' || &filetype is# 'json' ||
\ &filetype is# 'javascript'
Copy link
Contributor

Choose a reason for hiding this comment

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

This loose matching includes some patterns you might not have intended. Consider changing it to something like

&filetype is# 'html' || &filetype =~# '\mhtml\.'

Without the # the behavior subtly depends on the 'ignorecase' setting, and the loose .* will match on characters that aren't a literal dot, such as some weird htmlos filetype vim has that doesn't seem to be anything like actual HTML syntax.

elseif &filetype != ""
let l:cmd = l:cmd + ['--type', &filetype]
elseif &filetype
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this line here by accident? It's "harmless" but has no effect.

The `mustache/vim-mustache-handlebars` plugin sets the filetype to
either `html.handlebars` or `html.mustache`. Even though js-beautify
supports formatting for these files, the `--type` parameter must be
`html`.

This commit sets the `--type` parameter to `html` when the filetype
either is `html` or matches the regex `\mhtml\.`.
@abgeana
Copy link
Author

abgeana commented Mar 28, 2021

I addressed the two changes you requested and also squashed the commits to make history nicer. Let me know if this works.

@dbarnett
Copy link
Contributor

Thanks, merging!

@dbarnett dbarnett merged commit 048baf8 into google:master Mar 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants