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

add new rule django_block_translate_trimmed #137

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lb-
Copy link

@lb- lb- commented Apr 13, 2022

  • the rule will enforce the usage of trimmed when blocktranslate or blocktrans is in use
  • resolves the specific use case behind Ability to do simple checks on template tag content #131 - I think a generic rule would probably be way more complex and it is better to start with a specific one and add abstractions later

I had trouble running black, not sure if it is a config issue but no matter what I would put into Python files, black would not do anything so I just copy/pasted from the Black playground and back into the editor.

Additional items

  • test case for when trimmed is not the first argument to the blocktranslate tag when using multiple arguments
  • test case for when there are linebreaks between the open/close tags

@lb- lb- force-pushed the feature/block-translate-must-use-trimmed branch from b13c1b5 to f7e7d76 Compare April 13, 2022 06:07
@lb-
Copy link
Author

lb- commented Apr 13, 2022

Updated with a few more tests, not sure if I need to test malformed (e.g. unclosed) block tags.

@lb- lb- force-pushed the feature/block-translate-must-use-trimmed branch from f7e7d76 to d8be7a2 Compare April 13, 2022 10:09
("blocktrans", "plural", "endblocktrans"),
("block", "endblock"),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: this re-order is required because the approach is greedy, the first match is used.

So if we want to ensure that blocktranslate gets pulled in and it does not get treated as blocktrans or block we need to ensure that it is first.

Hope that makes sense.

Also blocktranslate is the Django 4.0 variant.

@lb- lb- force-pushed the feature/block-translate-must-use-trimmed branch from d8be7a2 to dce3e2d Compare April 13, 2022 10:11
Copy link

@loicteixeira loicteixeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👍

Thinking of other potential test cases:

  • trimmed is not the first argument to the blocktranslate tag when using multiple arguments
  • there are linebreaks between the open/close tags (but maybe this is something which is tested elsewhere in curlylint and do not need to be tested specifically here).

@lb-
Copy link
Author

lb- commented Nov 26, 2022

Loic - good call about the trimmed position - I'll try to resolve this soon. I'll update the description checklist to include this and the other item.

@loicteixeira @zerolab is there anything else blocking this PR?

@zerolab
Copy link

zerolab commented Nov 26, 2022

Nothing from me. Nice work @lb-!

- the rule will enforce the usage of `trimmed` when blocktranslate or blocktrans is in use
@lb- lb- force-pushed the feature/block-translate-must-use-trimmed branch from dce3e2d to 47eb4ef Compare November 28, 2022 22:18
@lb-
Copy link
Author

lb- commented Nov 28, 2022

Tests updated to include scenarios listed above, just need someone to trigger the workflow.

I was getting this error locally but it does not not seem to be related to the changes I have made.

self = <curlylint.cli_test.TestCLI testMethod=test_template_tags_cli_unconfigured_fails>

    def test_template_tags_cli_unconfigured_fails(self):
        runner = self.invoke_curlylint(
            1,
            ["--template-tags", "[]", "-"],
            input="<p>{% of a %}c{% elseof %}test{% endof %}</p>",
        )
>       self.assertIn(
            "Parse error: expected one of 'autoescape', 'block', 'blocktrans', 'comment', 'filter', 'for', 'if', 'ifchanged', 'ifequal', 'ifnotequal', 'not an intermediate Jinja tag name', 'spaceless', 'verbatim', 'with' at 0:17\tparse_error",
            runner.stdout_bytes.decode(),
        )
E       AssertionError: "Parse error: expected one of 'autoescape', 'block', 'blocktrans', 'comment', 'filter', 'for', 'if', 'ifchanged', 'ifequal', 'ifnotequal', 'not an intermediate Jinja tag name', 'spaceless', 'verbatim', 'with' at 0:17\tparse_error" not found in "\x1b[4m-\x1b[0m\n\x1b[2m0:17\x1b[0m\tParse error: expected one of 'autoescape', 'block', 'blocktrans', 'blocktranslate', 'comment', 'filter', 'for', 'if', 'ifchanged', 'ifequal', 'ifnotequal', 'not an intermediate Jinja tag name', 'spaceless', 'verbatim', 'with' at 0:17\tparse_error\n\n"

curlylint/cli_test.py:97: AssertionError
=============================================================== short test summary info ===============================================================
FAILED curlylint/cli_test.py::TestCLI::test_template_tags_cli_unconfigured_fails - AssertionError: "Parse error: expected one of 'autoescape', 'bloc...
======================================================= 1 failed, 91 passed, 1 skipped in 6.97s =======================================================
make: *** [Makefile:22: test] Error 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants