-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add template linting via djlint #25212
Conversation
709a04c
to
98142ea
Compare
It's nice that it found some issues to remedy, but I am very hesitant to add another language ecosystem just for this. |
I guess most people won't run this locally anyways, but I thought I at least enable them to do so. They don't have to install python to get gitea dev setup running. |
Erm… Have you seen how many PRs we already needed because there was an error in the templates that no one noticed? |
I don't disagree that this is helpful.
I'm not sure how true this is. It may be mostly true on many Linux systems, but I'm not sure about others. Furthermore, even if Python is installed, this requires installing Poetry as well, but...
I guess since it's not a requirement for dev I'm not exactly blocking, but it does feel a bit strange. |
I chose Poetry because I think it's the most sane python dependency manager currently. In short: The python dependency management ecosystems sucks, poetry is not ideal, but better then the other solutions. BTW, macOS also comes default with Python installed. Currently I have 3.9 as |
I just saw they support a experimental npm installation, so we might actually be able to skip the poetry dependency (it does require python and pip still). I will try that tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a simple "pip install" could be better if it works. Otherwise overall LGTM
It would leave the dependency unversioned, which in the long run will bring issues of inconsistent behaviour between versions, it's better to version-lock all dependencies. I will try the npm method. |
I do not think so. Or, use |
pip will pollute the global site-packages thought, I never use it because of that. |
Hmm yes, that's a problem and that's why pip is always used with virtualenv. Anyway, I think linting the templates is good and have voted my approval 😄 |
The npm module they publish is just a pip wrapper that will pollute global python packages, so I won't consider it and think the current solution is fine as-is. |
Decided to go with So the proper solution would have been to install pip to install pipx, pipx to install poetry and finally poetry to install the dependencies, but that chain was getting to insane for my tastes and Actions is mostly an isolated environment anyways where we don't need to worry so much about pip site-packages conflicts. |
Fix regression from #25212.
* upstream/main: (31 commits) Show OAuth2 errors to end users (go-gitea#25261) [skip ci] Updated translations via Crowdin Fix index generation parallelly failure (go-gitea#25235) Fix variable in template (go-gitea#25267) Add template linting via djlint (go-gitea#25212) Fix edit OAuth application width (go-gitea#25262) Use flex to align SVG and text (go-gitea#25163) GitHub Actions enhancements for frontend (go-gitea#25150) Add missing `v` in migrations.go (go-gitea#25252) Change form actions to fetch for submit review box (go-gitea#25219) Fix panic when migrating a repo from GitHub with issues (go-gitea#25246) Fix description of drop custom_labels migration (go-gitea#25243) Fix all possible setting error related storages and added some tests (go-gitea#23911) [skip ci] Updated translations via Crowdin Revert overflow: overlay (revert go-gitea#21850) (go-gitea#25231) Support changing labels of Actions runner without re-registration (go-gitea#24806) Improve AJAX link and modal confirm dialog (go-gitea#25210) Use inline SVG for built-in OAuth providers (go-gitea#25171) Disable `Create column` button while the column name is empty (go-gitea#25192) Fix profile render when the README.md size is larger than 1024 bytes (go-gitea#25131) ...
So I found this linter which features a mode for go templates, so I gave it a try and it did find a number of valid issue, like unbalanced tags etc. It also has a number of bugs, I had to disable/workaround many issues.
Given that this linter is written in python, this does add a dependency on
python
>= 3.8 andpoetry
to the development environment to be able to run this linter locally.e.g.
prefixes on placeholders are removed because the linter had a false-positive onplaceholder="e.g. cn=Search"
for theattr=value
syntax and it's not ideal anyways to writee.g.
into a placeholder because a placeholder is meant to hold a sample value.templates/repo/settings/options.tmpl
I simplified the logic to not conditionally create opening tags without closing tags because this stuff confuses the linter (and possibly the reader as well).