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

Solve/diagnose conflicting formatters #169

Closed
jspaezp opened this issue Sep 16, 2022 · 1 comment · Fixed by #173
Closed

Solve/diagnose conflicting formatters #169

jspaezp opened this issue Sep 16, 2022 · 1 comment · Fixed by #173
Labels
enhancement New feature or request

Comments

@jspaezp
Copy link
Contributor

jspaezp commented Sep 16, 2022

Started discussion in PR #162

#162 (comment)

Pitch: Passing over the formatters twice could help clear out problems where the work of one formatter is undone by another one.

The current implementation is mostly focused in finding that formatters conflict with each other (run again after a full pass)
and having it raise an error.

Current development branch ...
https://github.com/jspaezp/pydocstringformatter/tree/feature/twopass

Current broken tests ...

FAILED tests/test_config.py::TestStyleOption::test_style_in_toml - ValueError: Conflicting formatters: numpydoc-section-spacing, closing-quotes
FAILED tests/test_config.py::TestStyleOption::test_boolopt_in_toml - ValueError: Conflicting formatters: numpydoc-section-spacing, closing-quotes
FAILED tests/test_formatting.py::test_formatting[linewrap_summary-function_docstrings.py] - ValueError: Conflicting formatters: split-summary-body, final-period
FAILED tests/test_formatting.py::test_formatting[linewrap_summary-max_line_length_50.py] - ValueError: Conflicting formatters: split-summary-body, final-period
FAILED tests/test_formatting.py::test_formatting[numpydoc-numpydoc_regression.py] - ValueError: Conflicting formatters: numpydoc-section-spacing, closing-quotes

so .... is it of interest to add this changes? If anything it is a great debugging addition for development
should arguments be added to enable it?
should the error be more user-friendly?

If so and you have insights on those design aspects I will make this a PR

@Pierre-Sassoulas Pierre-Sassoulas added the enhancement New feature or request label Sep 16, 2022
@DanielNoord
Copy link
Owner

I haven't looked at your branch as I am on mobile, but I'm highly in favour of this change. I'll gladly help review a PR but sadly won't have time to work on this myself this week.

Some pointers to consider:

  • I think the maximum run count should be 2.
  • After the second run, a third run should occur. If we still create changes (note that these should ignore the --write flag but should only check if there are any potential change) we need to emit an error. black has a very elegant system where after there is an inconsistency aftwr 2/3 runs a pre-regenerated bug report is send to stdout. This can then be copied to an issue in GitHub which would allow us to resolve the issue.
  • We should test the issue template. So we need to write two incompatible test formatters and write a test where they fail to find a solution.

That's all I can think of for now. Let me know if you have any questions! 😊

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

Successfully merging a pull request may close this issue.

3 participants