Open lucaferranti opened 1 month ago
@abelsiqueira I marked this as draft as I need to update/fix tests, but feedback on the general direction welcome already.
What do you think about making the question about indentation conditional? I think it's a bit more coherent, since if someone wants to pick a styleguide, might be better for the generator not to encourage them immediately to introduce exceptions.
Another question, I think the linting will fail, because when I ran it locally it was sorting the lines of .JuliaFormater.toml.jinja
from
{% if StyleGuide == 'none' %}
indent = {{ Indentation }}
margin = 92
normalize_line_endings = "unix"
{% else %}
style = "{{ StyleGuide }}"
{% endif %}
into the nonsensical
indent = {{ Indentation }}
margin = 92
normalize_line_endings = "unix"
style = "{{ StyleGuide }}"
{% else %}
{% endif %}
{% if StyleGuide == 'none' %}
is there a way to prevent this?
Thanks for the PR!
The Indentation is used in other places - it also serves as the indentation of yaml, json and markdown (I think) files. So it should still be a required question. One possibility is to separate the Julia indentation from the rest of the code indentation - which I was thinking about doing anyway, because I like yaml 2 spaces even if Julia is 4 spaces. What do you think?
I don't use the styles, so I'm not sure if people follow them fully normally. It might be worth checking some random .JuliaFormatter.toml in the wild. Since I like indent 2, if I were to follow a style, I would probably still try to change it - i.e., add the indent even is style is selected. But we don't have to accommodate everything in the template, only the most expected (or recommended) practice. So probably it is fine to only have style
is a style is selected.
PS. I checked a few random files on GitHub searching for path:.JuliaFormatter.toml style
and it is a jungle of opinions. So let's keep the style as the only thing there if the user picks a style.
The sorting issue was unexpected, but I found an easy fix. The .pre-commit-config.yaml
hook file-contents-sorted
is expecting a regex for the files
argument, so we should just add a $
at the end of line 19.
The Indentation is used in other places - it also serves as the indentation of yaml, json and markdown (I think) files. So it should still be a required question. One possibility is to separate the Julia indentation from the rest of the code indentation - which I was thinking about doing anyway, because I like yaml 2 spaces even if Julia is 4 spaces. What do you think?
makes sense, I also find 2 spaces indentation more logical for yaml and other files. This decoupling allows to move the question about styleguide to code-quality.yml
, which is maybe a more suitable place? I don't think it's an essential question. One could also keep an additional conditional JuliaIndentation
question which is asked if the user selects to not adhere to any styleguide.
I'll do the changes and look into the tests later today
Yeah, I think I wanted to select indentation before deciding on strategy, because it will appear in the file regardless of strategy. But it makes more sense to have the style questions in code quality. I think we can move all the indentation questions there.
I also realised that if we split the Indentation, we should also adjust .editorconfig
and split the indent definition there.
If you think it is easier you can make a separate PR before this one only to split Indentation and move to code-quality.
Just a heads up, I've added a description
field in the copier yml files to further describe what the question is about. When you change the existing questions, you should have some easy-to-fix conflicts.
PR changes summary:
Related issues
Closes #424
Checklist
[x] I am following the contributing guidelines
[ ] Tests are passing
[x] Lint workflow is passing
[ ] Docs were updated and workflow is passing
[x] CHANGELOG.md was updated