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

Make "Found no matching notation to enable or disable." a warning #18670

Merged
merged 1 commit into from Feb 28, 2024

Conversation

proux01
Copy link
Contributor

@proux01 proux01 commented Feb 12, 2024

instead of an error

  • Added / updated test-suite.
  • Added changelog.
  • Added / updated documentation.
    • Documented any new / changed user messages.
    • Updated documented syntax by running make doc_gram_rsts.

@proux01 proux01 added kind: fix This fixes a bug or incorrect documentation. part: notations The notation system. labels Feb 12, 2024
@proux01 proux01 requested review from a team as code owners February 12, 2024 16:30
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Feb 12, 2024
@herbelin
Copy link
Member

herbelin commented Feb 12, 2024

The PR is nice for an "interactive"/"primary" use of Disable, but for a Imported deactivation on a non Imported notation, I believe something has to be done in the open_notation_toggle function, I recommend just ignoring the failure.

@herbelin
Copy link
Member

No, my comment is wrong. Let me think again.

@herbelin
Copy link
Member

herbelin commented Feb 12, 2024

I updated my comment. I would simply ignore the absence of a notation when deactivation comes from an Import.

Here is another variant of the Zulip example, showing a desynchronisation of imports:

(* comp.v *)
Definition comp {A B C} (f : B -> C) (g: A -> B) : A -> C := 
    fun a => f (g a).

Notation " g '\o' f " := (comp g f) (at level 40, left associativity).
Check ((@id nat) \o (@id nat)).
(* comp_utf.v *)
Require Import NotationTest.comp.

Disable Notation "\o".

Notation " g '∘' f " := (comp g f) (at level 40, left associativity).
(* comp_utf2.v *)
Require Import NotationTest.comp_utf.

Enable Notation "\o".

and

(* user.v*)
Require Import comp.
Require comp_utf.
Locate "\o". (* Notation " g '\o' f " := (comp g f) (default interpretation) *)
Require Import comp_utf2. (* Error: Found no matching notation to enable or disable *)

@proux01
Copy link
Contributor Author

proux01 commented Feb 27, 2024

@herbelin do you want to merge this or should I close it? (I unfortunately do'nt have time to do anything better in the foreseable future)

@herbelin
Copy link
Member

It brings something so I'm leaning towards merging.

@herbelin herbelin self-assigned this Feb 28, 2024
Copy link
Member

@herbelin herbelin left a comment

Choose a reason for hiding this comment

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

More convenient than an error.

@herbelin
Copy link
Member

@coqbot merge now

Copy link
Contributor

coqbot-app bot commented Feb 28, 2024

@herbelin: You cannot merge this PR because:

  • There is still a needs: full CI label.
  • No milestone has been set.

@herbelin herbelin modified the milestones: 8.20+rc1, 8.19.1 Feb 28, 2024
@herbelin
Copy link
Member

@coqbot run full ci

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Feb 28, 2024
@proux01
Copy link
Contributor Author

proux01 commented Feb 28, 2024

@herbelin unimath failure is unrelated (c.f. #18726 )

@herbelin
Copy link
Member

OK

@coqbot merge now

@coqbot-app coqbot-app bot merged commit c84de26 into coq:master Feb 28, 2024
5 of 7 checks passed
@coqbot-app coqbot-app bot added this to Request 8.19.1 inclusion in Coq 8.19 Feb 28, 2024
@proux01 proux01 deleted the warning_deactivate_nothing branch February 28, 2024 18:41
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Feb 29, 2024
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Feb 29, 2024
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Feb 29, 2024
coqbot-app bot added a commit that referenced this pull request Feb 29, 2024
Reviewed-by: proux01
Co-authored-by: proux01 <proux01@users.noreply.github.com>
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Feb 29, 2024
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Feb 29, 2024
(cherry picked from commit 01d0d0c)
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Feb 29, 2024
@coqbot-app coqbot-app bot moved this from Request 8.19.1 inclusion to Shipped in 8.19.1 in Coq 8.19 Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: fix This fixes a bug or incorrect documentation. part: notations The notation system.
Projects
Coq 8.19
Shipped in 8.19.1
Development

Successfully merging this pull request may close these issues.

None yet

2 participants