-
Notifications
You must be signed in to change notification settings - Fork 635
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
Addressing issue #4712: keyword detection preserves contiguous characters forming a valid identifier subsequence #16322
Conversation
1194cc4
to
d3e57fc
Compare
The job library:ci-fiat_crypto_legacy has failed in allow failure mode |
d3e57fc
to
17a56f8
Compare
🔴 CI failures at commit 17a56f8 without any failure in the test-suite ✔️ Corresponding jobs for the base commit 947398a succeeded ❔ Ask me to try to extract minimal test cases that can be added to the test-suite 🏃
|
The job library:ci-fiat_crypto_legacy has failed in allow failure mode |
17a56f8
to
fd8c819
Compare
@coqbot ci minimize ci-category_theory |
I am now running minimization at commit fd8c819 on requested target ci-category_theory. I'll come back to you with the results once it's done. |
Error: Could not minimize file /github/workspace/builds/coq/coq-failing/_build_ci/category_theory/Instance/Lambda.v (from ci-category_theory) (full log on GitHub Actions, cc @JasonGross) build log (truncated to last 26KiB; full 3.0MiB file on GitHub Actions Artifacts under
|
It seems ci-category_theory is failing in multiple PRs, my guess is that it's a problem on their end, and the bug minimizer cannot minimize it because the file structure changed. |
Can confirm it fails on master https://gitlab.com/coq/coq/-/jobs/2731873554 |
fd8c819
to
be9bad8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs a changelog entry (mostly containing the new documentation sentence).
Otherwise, modulo the few minor comments below, this LGTM and I'll merge by the end of next week if there is no further comment.
@herbelin could you rebase so as to avoid having a revert commit for a commit in the same PR? |
be9bad8
to
8758c0c
Compare
Rebased, changelog entry added, CI green, I'll merge by the end of the week if there is no further comment. |
8758c0c
to
1d61873
Compare
A contiguous sequence of characters must be fully inside or fully outside a keyword: no break of a keyword in the middle of a sequence of contiguous characters.
@coqbot merge now |
@proux01: You cannot merge this PR because:
|
@coqbot merge now |
Kind: wish granted, enhancement
This addresses two different kinds of examples:
Closes #4712
Depends on #16321