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 HTML formatting to pre-commit #966

Merged
merged 36 commits into from
Jul 12, 2023
Merged

Add HTML formatting to pre-commit #966

merged 36 commits into from
Jul 12, 2023

Conversation

michplunkett
Copy link
Collaborator

@michplunkett michplunkett commented Jul 10, 2023

Fixes issue

#239

Description of Changes

Added djLint to the pre-commit process and formatted files accordingly.

Tests and linting

  • This branch is up-to-date with the develop branch.
  • pytest passes on my local development environment.
  • pre-commit passes on my local development environment.

pre-commit output:

(openoversight) xxxxxx@MacBook-Air-5 OpenOversight % make lint
pre-commit run --all-files
trim trailing whitespace.................................................Passed
check docstring is first.................................................Passed
check that executables have shebangs.....................................Passed
check json...............................................................Passed
check for case conflicts.................................................Passed
check toml...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check xml................................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
check for broken symlinks............................(no files to check)Skipped
mixed line ending........................................................Passed
fix python encoding pragma...............................................Passed
pretty format json.......................................................Passed
fix requirements.txt.....................................................Passed
check blanket noqa.......................................................Passed
check for not-real mock methods..........................................Passed
check for eval().........................................................Passed
use logger.warning(......................................................Passed
Run isort to sort imports................................................Passed
Run pydocstyle...........................................................Passed
Do not use shebangs in non-executable files..............................Passed
flake8...................................................................Passed
black....................................................................Passed
djLint formatting........................................................Passed
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight % 

@michplunkett michplunkett self-assigned this Jul 10, 2023
@michplunkett michplunkett linked an issue Jul 10, 2023 that may be closed by this pull request
@michplunkett michplunkett changed the title Add HTML linting to CI Add HTML formatting to CI Jul 11, 2023
@michplunkett michplunkett marked this pull request as ready for review July 11, 2023 20:05
Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

What a neat find! Being able to lint the template files is a huge win. I have one question about some changed assertions, I'm curious if we could still preserve the check for content order.

Comment on lines 1682 to 1684
assert any("{}".format(unit.descrip) in token for token in filter_list)
assert any("<dd>" in token for token in filter_list)
assert any("</dd>" in token for token in filter_list)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The changes to these assertions maybe feel like we aren't testing what we previously were (since order no longer matters with the new tests). Is it possible to change the assertions so they test the same thing even if there is whitespace in between?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll give it a shot, yup! It's replicated in the next PR (the image filtering one), so I'll make sure to make whatever change I make on this PR on that one as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome, thank you! I wonder if we could do something like combined = "".join([token.strip() for token in filter_list]), then maybe the assertion can remain the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That sounds much simpler than the regex solution I was thinking of trying. I'll give it a shot tomorrow and respond back here.

Copy link
Collaborator Author

@michplunkett michplunkett Jul 12, 2023

Choose a reason for hiding this comment

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

I tried your solution and my solution of filter_list = [token.replace(" ", "").replace("\n", "") for token in filter_list to no avail.

The problem was that between the <dd> elements you had a starting line break and A LOT of spaces. To address that, I created this function and created all of the filter_lists with it.

def normalize_tokens_for_comparison(html_str: str, split_str: str):
  """Remove new lines, leading, and closing spaces between <dd> elements in
  formatted HTML."""
  parsed_list = html_str.data.decode(ENCODING_UTF_8).split(split_str)[1:]
  parsed_list = [re.sub(r"<dd>\n\s+", "<dd>", token) for token in parsed_list]
  parsed_list = [re.sub(r"\n\s+</dd>", "</dd>", token) for token in parsed_list]
  return parsed_list

I've since added it to both branches.

@michplunkett michplunkett mentioned this pull request Jul 12, 2023
3 tasks
@michplunkett michplunkett changed the title Add HTML formatting to CI Add HTML formatting to pre-commit Jul 12, 2023
Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

Fantastic!

@michplunkett michplunkett merged commit 7be3e2f into develop Jul 12, 2023
@michplunkett michplunkett deleted the add_html_linter branch July 12, 2023 18:34
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 6, 2023
lucyparsons#239

Added [`djLint`](https://www.djlint.com/docs/languages/jinja/) to the
`pre-commit` process and formatted files accordingly.

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

`pre-commit` output:
```console
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight % make lint
pre-commit run --all-files
trim trailing whitespace.................................................Passed
check docstring is first.................................................Passed
check that executables have shebangs.....................................Passed
check json...............................................................Passed
check for case conflicts.................................................Passed
check toml...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check xml................................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
check for broken symlinks............................(no files to check)Skipped
mixed line ending........................................................Passed
fix python encoding pragma...............................................Passed
pretty format json.......................................................Passed
fix requirements.txt.....................................................Passed
check blanket noqa.......................................................Passed
check for not-real mock methods..........................................Passed
check for eval().........................................................Passed
use logger.warning(......................................................Passed
Run isort to sort imports................................................Passed
Run pydocstyle...........................................................Passed
Do not use shebangs in non-executable files..............................Passed
flake8...................................................................Passed
black....................................................................Passed
djLint formatting........................................................Passed
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight %
```
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 25, 2023
lucyparsons#239

Added [`djLint`](https://www.djlint.com/docs/languages/jinja/) to the
`pre-commit` process and formatted files accordingly.

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

`pre-commit` output:
```console
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight % make lint
pre-commit run --all-files
trim trailing whitespace.................................................Passed
check docstring is first.................................................Passed
check that executables have shebangs.....................................Passed
check json...............................................................Passed
check for case conflicts.................................................Passed
check toml...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check xml................................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
check for broken symlinks............................(no files to check)Skipped
mixed line ending........................................................Passed
fix python encoding pragma...............................................Passed
pretty format json.......................................................Passed
fix requirements.txt.....................................................Passed
check blanket noqa.......................................................Passed
check for not-real mock methods..........................................Passed
check for eval().........................................................Passed
use logger.warning(......................................................Passed
Run isort to sort imports................................................Passed
Run pydocstyle...........................................................Passed
Do not use shebangs in non-executable files..............................Passed
flake8...................................................................Passed
black....................................................................Passed
djLint formatting........................................................Passed
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight %
```
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Oct 5, 2023
lucyparsons#239

Added [`djLint`](https://www.djlint.com/docs/languages/jinja/) to the
`pre-commit` process and formatted files accordingly.

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

`pre-commit` output:
```console
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight % make lint
pre-commit run --all-files
trim trailing whitespace.................................................Passed
check docstring is first.................................................Passed
check that executables have shebangs.....................................Passed
check json...............................................................Passed
check for case conflicts.................................................Passed
check toml...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check xml................................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
check for broken symlinks............................(no files to check)Skipped
mixed line ending........................................................Passed
fix python encoding pragma...............................................Passed
pretty format json.......................................................Passed
fix requirements.txt.....................................................Passed
check blanket noqa.......................................................Passed
check for not-real mock methods..........................................Passed
check for eval().........................................................Passed
use logger.warning(......................................................Passed
Run isort to sort imports................................................Passed
Run pydocstyle...........................................................Passed
Do not use shebangs in non-executable files..............................Passed
flake8...................................................................Passed
black....................................................................Passed
djLint formatting........................................................Passed
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight %
```
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Oct 9, 2023
lucyparsons#239

Added [`djLint`](https://www.djlint.com/docs/languages/jinja/) to the
`pre-commit` process and formatted files accordingly.

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

`pre-commit` output:
```console
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight % make lint
pre-commit run --all-files
trim trailing whitespace.................................................Passed
check docstring is first.................................................Passed
check that executables have shebangs.....................................Passed
check json...............................................................Passed
check for case conflicts.................................................Passed
check toml...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check xml................................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
check for broken symlinks............................(no files to check)Skipped
mixed line ending........................................................Passed
fix python encoding pragma...............................................Passed
pretty format json.......................................................Passed
fix requirements.txt.....................................................Passed
check blanket noqa.......................................................Passed
check for not-real mock methods..........................................Passed
check for eval().........................................................Passed
use logger.warning(......................................................Passed
Run isort to sort imports................................................Passed
Run pydocstyle...........................................................Passed
Do not use shebangs in non-executable files..............................Passed
flake8...................................................................Passed
black....................................................................Passed
djLint formatting........................................................Passed
(openoversight) xxxxxx@MacBook-Air-5 OpenOversight %
```
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.

Add HTML formatting and linting to pre-commit
2 participants