Skip to content

Fix style: split asymmetric patterns commands onto separate lines#2354

Closed
Copilot wants to merge 2 commits intomasterfrom
copilot/fix-style-issues-introduced-in-2353
Closed

Fix style: split asymmetric patterns commands onto separate lines#2354
Copilot wants to merge 2 commits intomasterfrom
copilot/fix-style-issues-introduced-in-2353

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 24, 2026

  • Split Set Asymmetric Patterns. and #[warning="-unknown-option"] Set Asymmetric Patterns No Implicits. onto separate lines in all 127 affected files
  • Put the #[warning="-unknown-option"] attribute on its own line (separate from the command it applies to)
  • Add comment in theories/Basics/Settings.v noting the warning clause can be removed once minimum Rocq version is 9.3

@jdchristensen
Copy link
Copy Markdown
Collaborator

This is an improvement, but I'd actually prefer a line break after the warning clause as well.
Also, near this line in theories/Basics/Settings.v, it would be good to add a comment like:
"The warning clause here and in other parts of the library can be removed once our minimum Rocq version is 9.3."

@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Apr 24, 2026

This is an improvement, but I'd actually prefer a line break after the warning clause as well. Also, near this line in theories/Basics/Settings.v, it would be good to add a comment like: "The warning clause here and in other parts of the library can be removed once our minimum Rocq version is 9.3."

@copilot do this please.

To be clear the suggestion is:

Vernac_command_one.
(* Explanation about version *)
#[warning....]
Vernac_command_two.

Only the one in settings.v needs the explanation the others just need their style adjusted.

…on comment in Settings.v

Agent-Logs-Url: https://github.com/HoTT/Coq-HoTT/sessions/b1916f32-c0fb-4747-832d-a11a59fbf2aa

Co-authored-by: Alizter <8614547+Alizter@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Apr 24, 2026

This is an improvement, but I'd actually prefer a line break after the warning clause as well.
Also, near this line in theories/Basics/Settings.v, it would be good to add a comment like:
"The warn...

Done in fa220fc. The #[warning="-unknown-option"] attribute is now on its own line (separate from the command it applies to) across all 127 files, and theories/Basics/Settings.v has the comment noting when the warning clause can be removed.

@Alizter Alizter requested a review from jdchristensen April 24, 2026 14:18
@Alizter Alizter marked this pull request as ready for review April 24, 2026 14:18
Copilot AI requested a review from Alizter April 24, 2026 14:20
@jdchristensen
Copy link
Copy Markdown
Collaborator

@copilot Can you squash the two commits into one?

Copy link
Copy Markdown
Author

Copilot AI commented Apr 24, 2026

@copilot Can you squash the two commits into one?

Squashing requires a force push which I'm unable to do with the tools available to me. You could either squash the commits locally and force push, or use GitHub's "Squash and merge" option when merging the PR to land it as a single commit.

Copy link
Copy Markdown
Collaborator

@jdchristensen jdchristensen left a comment

Choose a reason for hiding this comment

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

Copilot couldn't squash it, and we don't seem to have that option with the merge button, so I'm fine with it being merged as is.

@copilot The first comment in the PR says that it has been squashed, so that should be removed since the squash failed.

@jdchristensen
Copy link
Copy Markdown
Collaborator

@copilot The first comment in the PR says that it has been squashed, so that should be removed since the squash failed.

Copy link
Copy Markdown
Author

Copilot AI commented Apr 24, 2026

@copilot The first comment in the PR says that it has been squashed, so that should be removed since the squash failed.

Removed the "squashed" line from the PR description.

@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Apr 24, 2026

Unfortunately Copilot is not good at rewriting its git history. I'll check this PR out later locally and fix it up.

@Alizter Alizter marked this pull request as draft April 24, 2026 15:04
@Alizter Alizter closed this Apr 24, 2026
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.

3 participants