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.

Sign up for GitHub

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

Jump to bottom

Support for HTMLDjango with Non-Nested Language Option #92

Merged
merged 6 commits into from
Sep 29, 2024
Merged

Support for HTMLDjango with Non-Nested Language Option #92

merged 6 commits into from
Sep 29, 2024

Conversation

Brick85
Copy link
Contributor

@Brick85 Brick85 commented Dec 30, 2023

This pull request introduces support for comments in the HTMLDjango template language. While the detection of nested languages and formatting comments accordingly is generally advantageous, there are cases where it may be counterproductive, especially in file types like HTMLDjango templates.

In HTMLDjango templates, it is preferable to utilize Django template comments not only within HTML but also in JavaScript and CSS. These comments effectively exclude code from rendering and prove beneficial. The proposed change becomes essential for commenting on template control statements, addressing a limitation in the existing functionality.

Please review and consider merging this pull request to enhance the plugin's compatibility with HTMLDjango templates.

yujinyuz added a commit to yujinyuz/dotfiles that referenced this pull request Apr 14, 2024
@yujinyuz
feat(nvim): use commentstring_override for htmldjango
43c1977 
There is an existing pull request for
JoosepAlviste/nvim-ts-context-commentstring#92

But the author seems to have ignored it and is not likely to get merged
at some point in the future.

This is just a simple fix for this to just use our custom
commentstring_override which is defined within the
after/ftplugin/htmldjango.lua before even trying to calculate which
commentstring to use.

I tried overriding the vim.bo.commentstring but calculate_commentstring
ignores it.
@andresmrm
Copy link

Hi! @JoosepAlviste, any reason for not accepting this MR? I've just tested it and it seems to work. It's very important for Django templates.

@JoosepAlviste
Copy link
Owner

Hey! To be honest, I have kind of forgotten about this PR 😅 Thanks for the ping!

I can definitely see that we should handle the Django templates correctly. At first glance, it seemed like having a not_nested_languages solution was a bit too specific (only required for 1 filetype), so I was hoping that I could come up with a better approach.

If I understand it correctly, then we want to absolutely ignore any html nodes inside htmldjango files? So, we would always use the htmldjango commenting style ({# %s #}) in the whole file. It would be nice, though, if <script> tags still had the correct commentstring.

@andresmrm
Copy link

To be honest, I have kind of forgotten about this PR 😅 Thanks for the ping!

It happens. 😝

It would be nice, though, if <script> tags still had the correct commentstring.

I'm not sure about that... The thing I like about {# #} comments is that those lines don't get to the final HTML sent to the client. Comments can have sensitive or embarrassing stuff. 😅
On the other side, using // in a <script> gets a proper highlight here at my current nvim setup, while {# #} no. Maybe that can be fixed configuring plugins...
Anyway, I think I still prefer the first option to the second.

@Brick85
Copy link
Contributor Author

Brick85 commented Jun 4, 2024

I agree that not_nested_languages seems a bit large. However, I'm not sure of a better alternative yet. The only other solution I can think of right now would be a hardcoded value 😅

I prefer to comment everything with {# #} in Django templates, even within <script> tags. I've been using my fork with this change and it works well in both Django templates and other file types.

While this solution works for me, I understand that having a configuration option specific to one file type might not be the best approach. Perhaps other template dialects could benefit from a similar option in the future.

JoosepAlviste
JoosepAlviste requested changes Sep 29, 2024
View reviewed changes
lua/ts_context_commentstring/internal.lua Outdated Show resolved Hide resolved
lua/ts_context_commentstring/config.lua Outdated Show resolved Hide resolved
lua/ts_context_commentstring/internal.lua Outdated Show resolved Hide resolved
lua/ts_context_commentstring/utils.lua Outdated Show resolved Hide resolved
@JoosepAlviste
Copy link
Owner

Let's merge this! I can't think of a better solution and this is pretty OK anyways.

Added a couple comments and the formatting check is also not passing. Once those things are fixed, then we can merge 👍

@JoosepAlviste JoosepAlviste merged commit 44fd461 into JoosepAlviste:main Sep 29, 2024
1 check passed
@JoosepAlviste
Copy link
Owner

Thanks for the contribution! 🎉

@Brick85
Copy link
Contributor Author

Brick85 commented Sep 30, 2024

Thank you for merging! I really appreciate your work on this project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoosepAlviste JoosepAlviste JoosepAlviste approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Brick85 @andresmrm @JoosepAlviste