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.

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

Lint: Better indentation handling #5767

Closed
wxtim opened this issue Oct 12, 2023 · 5 comments
Closed

Lint: Better indentation handling #5767

wxtim opened this issue Oct 12, 2023 · 5 comments
Assignees
Labels
could be better Not exactly a bug, but not ideal.

Comments

@wxtim
Copy link
Member

wxtim commented Oct 12, 2023

Problem

Cylc Lint's indentation handling only compares number of [ to indentation for sections.

Now Lint has access to the Spec it should be able to infer the correct indentation for any non-duplicate section. Even then if sections like 'handlers' should be ok because both workflow and task handlers should have the same indentation.

@wxtim wxtim self-assigned this Oct 12, 2023
@wxtim wxtim added this to the cylc-8.3.0 milestone Oct 12, 2023
@wxtim wxtim added the could be better Not exactly a bug, but not ideal. label Oct 12, 2023
@oliver-sanders
Copy link
Member

If we implement the logic to identify invalid sections e.g. [schdulng], then this should ensure the number of ['s is correct, which should ensure the indentation is correct.

@oliver-sanders oliver-sanders modified the milestones: cylc-8.3.0, cylc-8.x Oct 12, 2023
@wxtim
Copy link
Member Author

wxtim commented Oct 13, 2023

If we implement the logic to identify invalid sections e.g. [schdulng], then this should ensure the number of ['s is correct, which should ensure the indentation is correct.

We already check whether the number of [ matches the indentation, what we don't check is whether key value pairs are indented correctly; Lint will not spot:

[scheduler]
        allow implicit tasks = false  # Eight spaces

@oliver-sanders
Copy link
Member

It's doable, but tricky because of Jinja2. E.G. a lot of people will do this:

# the indentation is nicer before processing
{% for x in y %}
    {{ x }}
{% endfor %}

Rather than this:

# the indentation is nicer after processing
{% for x in y %}
{{ x }}
{% endfor %}

@wxtim
Copy link
Member Author

wxtim commented Oct 16, 2023

It's doable, but tricky because of Jinja2. E.G. a lot of people will do this:

In the first instance I would suggest that we should test whether indent_size % 4 == 0 - I'm sure I tested this, but it seems to have gone missing.

In the second instance, we can at least say that a given item should not be less indented than X - e.g. initial cycle point should be indented at least 4 spaces.

@wxtim
Copy link
Member Author

wxtim commented Jan 16, 2024

#5772 Is probably the best we can do for now?

@wxtim wxtim closed this as completed Mar 4, 2024
@MetRonnie MetRonnie removed this from the cylc-8.x milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
could be better Not exactly a bug, but not ideal.
Projects
None yet
Development

No branches or pull requests

3 participants