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

Bugfix: Unbalanced autolink brackets #616

Merged

Conversation

inexorabletash
Copy link
Contributor

@inexorabletash inexorabletash commented Mar 21, 2024

Introduced by me in 1d1b531 c761733. Oops.


Preview | Diff

@inexorabletash
Copy link
Contributor Author

@anssiko - maybe you can review this trivial fix?

And... maybe we can look into tools to catch these? Options that come to mind:

  • Pre-commit script that greps for bad patterns in the Bikeshed source (would have caught this one) - pro: easy to author; con: hard to maintain and regexes are non-trivial; can't catch post-rendering problems
  • Local script executed by the Makefile that greps for bad patterns in the HTML output (plausible) - pro: can catch post-rendering problems; con: validating HTML with grep is also non-trivial
  • Local script executed by the Makefile that uses a DOM parser (like BeautifulSoup) to lint the rendered output - pro: can catch nearly everything; con: even more difficult to author
  • Script in the rendered HTML page itself that introspects and emits visible / console warnings - pro: JS is easy, can operate on the rendered output; con: can't automate, requires someone to look at output

I'm happy to tackle whichever if it'll improve our reliability and velocity.

Copy link
Member

@anssiko anssiko left a comment

Choose a reason for hiding this comment

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

LGTM!

@anssiko
Copy link
Member

anssiko commented Mar 21, 2024

Do you know if it’d be feasible for Bikeshed to catch these?

@anssiko anssiko merged commit 2dff633 into webmachinelearning:main Mar 21, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Mar 21, 2024
SHA: 2dff633
Reason: push, by anssiko

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@inexorabletash
Copy link
Contributor Author

Do you know if it’d be feasible for Bikeshed to catch these?

There's been some discussion but the Bikeshed backlog is very, very long so I would not expect a quick fix.

@anssiko
Copy link
Member

anssiko commented Mar 22, 2024

@inexorabletash thanks for checking. Regarding the options you listed, I'll let you pick your favourite in consultation with the editors. Any improvements that makes editing the spec less error prone, more enjoyable, even more fun, is always welcome!

Copy link
Contributor

@huningxin huningxin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

@inexorabletash inexorabletash deleted the bugfix-autolink-typos branch April 4, 2024 18:22
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