Linters for HTML, JS

[koeaw]

I've looked at the precommit config used for APIS vanilla but didn't see anything related to linting/code formatting for HTML or JavaScript in there.

Same as how we agreed on using black for Python, I think it would be useful if we all used consistent formatting for Django templates (and related files) also.

Whenever I've refactored templates now, for example, I've always reduced the indentation to 2 spaces first thing, because we currently seem to use old-school 4-space indentation everywhere, which very quickly makes HTML & JS files hard to parse/makes an IDE wrap lines a lot sooner. (2 spaces is best practice nowadays.)

[koeaw]

Django Packages suggests three tools for template linting: https://djangopackages.org/grids/g/template-linters/

[koeaw]

It may make sense to look at how other (larger) projects handle this issue.

From Wagtail's HTML guidelines:

We use djhtml for formatting and Curlylint for linting.

[koeaw]

Relatedly: acdh-oeaw/apis-core-rdf#42

I think a linter solution for templates – once we have one – should be applied to all relevant files in one go, just like the other issue proposes for Python code + black.

Manual updates result in overhead IME, because I'll often only think of reformatting a template for readability after I've already started working on it for other reasons...

[SteffRhes]

I agree. Please go ahead with this.

[koeaw]

Tagged for next team discussion in case someone else already has hands-on experience with this.

[sennierer]

@richardhadden to do some research and come up with suggestions for nex jfx

[koeaw]

Following on from my last comment in https://github.com/acdh-oeaw/apis-core-rdf/pull/116, which contains a GitHub workflow for reformatting templates using djLint (+ reformatted templates):

I ended up installing the package to work around the merge conflicts & to take a closer look at its capabilities in general because it would be preferable if it could be used with apis-rdf-devops / for local development

... Doing so, I noticed one thing it forces on users which is detrimental to readability, but for opting out of which there's already an open feature request, which is solvable (to the extent it's relevant for us) relatively easily, for which I'd want to submit a PR.

The only blocker for using DJLint with apis-rdf-devops seems to be the pinned PyYAML version in apis-core-rdf. It caused Poetry to get stuck in a loop for me as I was trying to add djlint to apis-rdf-devops's dev dependencies group, which I worked around locally by manually changing the version no.[*] in the pyproject.toml file. But I don't know if there's a way to test if/how this would affect other dependencies.

The GitHub workflow doesn't seem to use Poetry to install the package anyway, so it looks like we could use it independently, but I would suggest to use the same set of djLint settings across all repos. I'd also opt to put the settings in .djlintrc files to keep the pyproject.toml clean / for greater transparency (probably even more applicable in the case of the GitHub workflow, which pip installs the package rather than using Poetry).

[*] more precisely, I simply set it from "^5.3.1" to ">=5.3.1"

[b1rger]

I noticed one thing it forces on users which is detrimental to readability, but for opting out of which there's already an open feature request, which is solvable

and that would be?

The only blocker for using DJLint with apis-rdf-devops seems to be the pinned PyYAML version

I don't think it makes sense to implement this in apis-rdf-devops, given #59 - when that is done there won't be any template files left and this repository will be what its names says it is: a devops repo.

Also, the PyYAML version is not a blocker for using DJLint. Only if you want to use the same Python environment for both. But the PyYAML dependency is actually a very easy fix, given it is only used by RDFParser which is not working right now anyway, so we can simply update the dependency.

The GitHub workflow doesn't seem to use Poetry to install the package anyway, so it looks like we could use it independently, but I would suggest to use the same set of djLint settings across all repos. I'd also opt to put the settings in .djlintrc files to keep the pyproject.toml clean / for greater transparency

I am strongly against spreading configuration over multiple files if we have the chance to keep it in a single location. Most modern Python packaging tools are supporting pyproject.toml or are working on implementing support. Using one commonly used configuration file gives us the advantage of having one single source of truth. It also means we don't have to remember every configuration file location for every tool we ever want to use. (Aside from .djlintrc being json...)

(probably even more applicable in the case of the GitHub workflow, which pip installs the package rather than using Poetry).

I don't get what the way a software is installed has to do with where it gets its configuration settings from.

[koeaw]

and that would be?

Its condensation "feature".

I don't think it makes sense to implement this in apis-rdf-devops, given https://github.com/acdh-oeaw/apis-rdf-devops/issues/59 - when that is done there won't be any template files left and this repository will be what its names says it is: a devops repo.

Ok, but how would we use the tool for local development then? Analogous to black. Which is also part of apis-rdf-devops' dev dependencies, from where it can be used for all 3 repos. I'm assuming our own templates / template overrides will continue to reside in apis-ontologies, so what's the idea for how an HTML linter/formatter would be used so it reaches all relevant files across all repos/apps?

I am strongly against spreading configuration over multiple files if we have the chance to keep it in a single location.

Except we won't if we want to make use of djLint's rules, which need to go in a separate yml file. Which is similarly named to the config file, so those two files end up sitting right next to each other / are recognisable as belonging together.

Most modern Python packaging tools are supporting pyproject.toml or are working on implementing support.

Ok, I wasn't aware of that, but also re:

Using one commonly used configuration file gives us the advantage of having one single source of truth. It also means we don't have to remember every configuration file location for every tool we ever want to use.

I didn't see any existing configs in the file, this was in fact the first time I've encountered settings being put there, so I had to go look up what's up with that.

So it's not like my suggestion of using these djLint-specific files is me working against any established conventions for our projects or whatever, and I object to the insinuation it was outlandish. If this should be a rule, it'd help to officially make it one; there are other team members besides me who didn't use Poetry at all either until our recently-ish switch to it, and who might have looked into it even less than me since then.

I don't get what the way a software is installed has to do with where it gets its configuration settings from.

It doesn't, I was expressly mentioning transparency, which is relevant to humans, not machines. If half the team doesn't know pyproject.toml is used to store settings for random tools that don't have any connection to Poetry, how would they know to go looking for them there if they wanted to change something about the workflow.

[b1rger]

Ok, but how would we use the tool for local development then?

I don't know what that question is aiming at. djlint is a tool, you install it using your preferred method of software installation, you integrate it in your personal workflow like you do with any other tools.

Analogous to black. Which is also part of apis-rdf-devops' dev dependencies, from where it can be used for all 3 repos. I'm assuming our own templates / template overrides will continue to reside in apis-ontologies, so what's the idea for how an HTML linter/formatter would be used so it reaches all relevant files across all repos/apps?

If we put configuration and development dependencies only in umbrella repositores, but not in the repositories themselves, people who don't use the umbrella repositories simply won't use the configuration. Especially for a linter, which we also use in Github and which will lead to failing workflows, it is important that the configuration resides in the repository, so contributers know which linter settings to use.

I don't know why black is listed in the devops repos pyproject.toml - probably because there are Python files in that repository? But it is also part of apis-core-rdf (with a different version and the poetry group is defined differently). And maybe it makes sense to remove black when webpage is its own project

There is no plan to have linter settings defined in one central repository. What about other projects that are not part of this umbrella repository, like apis-bibsonomy or apis-highlighter? People working on apis tend to work on these too. What we can do, is create a central workflow for running the linter, that can be included in the repostiories. But the configuration itself should still be part of the repos, so they have the liberty of setting their own preferences.

I am strongly against spreading configuration over multiple files if we have the chance to keep it in a single location.

Except we won't if we want to make use of djLint's rules, which need to go in a separate yml file. Which is similarly named to the config file, so those two files end up sitting right next to each other / are recognisable as belonging together.

Well, the djlint documentation says:

Create a file .djlint_rules.yaml alongside your pyproject.toml

So, apparently they assume people using pyproject.toml. And given that we as contributors are not even able to find some basic common rulesets for which settings the linter should initially use, I would advise against getting into the discussion about addition rules.

I didn't see any existing configs in the file, this was in fact the first time I've encountered settings being put there, so I had to go look up what's up with that.

Because we don't use project wide settings yet. black configuration could be part of pyproject.toml, but "Black is all about sensible defaults" ;)

So it's not like my suggestion of using these djLint-specific files is me working against any established conventions for our projects or whatever, and I object to the insinuation it was outlandish.

That was not my intention at all, I'm sorry if that came across like that!

If this should be a rule, it'd help to officially make it one; there are other team members besides me who didn't use Poetry at all either until our recently-ish switch to it, and who might have looked into it even less than me since then.

I don't get what the way a software is installed has to do with where it gets its configuration settings from.

It doesn't, I was expressly mentioning transparency, which is relevant to humans, not machines. If half the team doesn't know pyproject.toml is used to store settings for random tools that don't have any connection to Poetry, how would they know to go looking for them there if they wanted to change something about the workflow.

Hm, maybe I'm stating the obvious, but I'm not sure what the connection to poetry is: pyproject.toml is not a poetry configuration file. pyproject.toml is a file where various tools (poetry being one random tool like the others) are storing their configuration. So there is no special connection between poetry and pyproject.toml- thats why it is using the [tool.poetry] sections, like black is using the [tool.black] sections and djlint would be using the [tool.djlint] sections. And there are some standardized metadata fields which are tool independent.

[koeaw]

Closing this as this repo is being phased out and all further discussion of linters for HTML, CSS and JS should probably happen over in apis-core-rdf. The latter's files got formatted once, I believe, though I'm not sure if we're currently enforcing formatting on new/changed files included in PRs via GitHub actions.