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.

Sign up for GitHub

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

Jump to bottom

replace triple-quotes by modifying tokens #144

Merged
merged 18 commits into from
Sep 12, 2022
Merged

replace triple-quotes by modifying tokens #144

merged 18 commits into from
Sep 12, 2022

Conversation

keewis
Copy link
Owner

@keewis keewis commented Sep 10, 2022 •
edited
Loading

This is hopefully much more robust than all of the previous attempts because we're actually extracting just the triple-quoted strings from the reformatted code, and before adding prompts.

  • Closes TokenError on second run #142
  • Tests added
  • Passes pre-commit run --all-files
  • User visible changes (including notable bug fixes) are documented in changelog.rst

@keewis
Copy link
Owner Author

keewis commented Sep 10, 2022 •
edited
Loading

@jugmac00, could you try running this on your codebase with the state from before #142? That might help me confirm if this actually fixes the triple-quote reconstruction algorithm.

@keewis keewis mentioned this pull request Sep 10, 2022
Closed
@jugmac00
Copy link

@keewis Thanks for the PR!

Unfortunately, it looks like it introduced some new issues - but probably related.

Apologies that I cannot investigate further, as today is super packed both from work and personal life.

The new (or at least newly formatted) error messages:

❯ pre-commit run blackdoc --all-files
[WARNING] The 'rev' field of repo 'https://github.com/keewis/blackdoc' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
blackdoc.................................................................Failed
- hook id: blackdoc
- exit code: 123

error: cannot format /home/jugmac00/launchpad/launchpad/lib/lp/services/database/sort_sql.py: Cannot parse: 95:43: EOF in multi-line string
error: cannot format /home/jugmac00/launchpad/launchpad/lib/lp/services/doc/pidfile.rst: Cannot parse: 62:5: cmd = "'''import os.path, sys
error: cannot format /home/jugmac00/launchpad/launchpad/lib/lp/testing/doc/pagetest-helpers.rst: Cannot parse: 106:24: content = "'''<html id="root">
error: cannot format /home/jugmac00/launchpad/launchpad/lib/lp/translations/doc/pofile.rst: Cannot parse: 274:19: new_header_string = "'''roject-Id-Version: es
error: cannot format /home/jugmac00/launchpad/launchpad/lib/lp/translations/utilities/doc/gettext_po_parser.rst: Cannot parse: 93:19: new_header_string = "'''roject-Id-Version: es
Oh no! 💥 💔 💥
2953 files left unchanged, 5 files fails to reformat.
All done! ✨ 🍰 ✨
1045 files left unchanged.

Please note that this time 5 files are affected, not 11 like with blackdoc from PyPI.

You can reproduce the issue like the following:

git clone https://git.launchpad.net/launchpad
git checkout 41b925e
blackdoc lib/lb
blackdoc lib/lb

or with the appropriate pre-commit configuration:

-   repo: https://github.com/keewis/blackdoc
    rev: triple-quotes
    hooks:
    -   id: blackdoc
        args: ["-l", "78"]
        exclude: ^doc/.*

@keewis
Copy link
Owner Author

keewis commented Sep 12, 2022 •
edited
Loading

great, thanks a lot for the instructions and the examples. I managed to track down a off-by-one error, which I fixed in the latest commits (and there's actually tests for this so hopefully there won't be any regressions in the future)

@jugmac00
Copy link

Works like a charm - thanks so much! 🚀

@keewis
Copy link
Owner Author

keewis commented Sep 12, 2022

thanks a lot for confirming, @jugmac00!

That still leaves us with #145, but blackdoc has always worked that way so I don't think it's urgent.

@keewis keewis merged commit e026b13 into main Sep 12, 2022
@keewis keewis deleted the triple-quotes branch September 12, 2022 21:34
@keewis keewis linked an issue Sep 12, 2022 that may be closed by this pull request
triple-quote reconstruction algorithm #139
Closed
@keewis keewis mentioned this pull request Sep 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jugmac00 jugmac00 jugmac00 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TokenError on second run triple-quote reconstruction algorithm
2 participants
@keewis @jugmac00