-
Notifications
You must be signed in to change notification settings - Fork 91
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
Fix comment syntax in set.mm #3521
Conversation
This fixes some errors from the new doubled underscore code in metamath-knife.
3c7f75e
to
62ff0fc
Compare
@@ -743770,7 +743770,7 @@ ordered pairs of real numbers (the cartesian product of the real | |||
less than the first coordinate of the other point, or the first | |||
coordinates of both points are equal and the second coordinate of the | |||
first point is less than the second coordinate of the other point: | |||
(a,b) <_ (x,y) :<-> ( a<x \/ ( a=x /\ b<_y) ). (Contributed by AV, |
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.
Wouldn't the better solution be to use "math mode" here?
I.e. put the formula within back tick escapes, and space out each symbol.
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.
Maybe we should ask @avekens . There isn't a math symbol called :<->
and to be honest I'm not completely sure how well the concepts here map to set.mm notions of ordered pair, no-greater-than of extended reals, etc.
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.
I guess this means "defined as", just like ":=" also sometimes means "defined as".
But yes, the best is to ask @avekens .
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 should have been a semiformal definition of a lexicographical ordering. Maybe this "formula" can be replaced by <. a , b >. <_ <. x , y >.
iff ( a < x \/ ( a = x /\ b <_ y ) )
.
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.
I've put that change into #3527
Thanks for all the actions (and sorry for the mess). Another action to avoid this in the future is to include a run of metamath-knife on the latest |
Actually, besides only running on set.mm's "master" branch, it seems that in its CI, metamath-knife does not run the "verify markup" and "verify typesetting" options. |
Looking at the errors generated by
|
metamath/metamath-knife#130 now also includes HTML mode fixes. Otherwise I think the summary is correct. I do think we should not silence the warning on undoubled |
Ah, I suspected there must be something like that, since I don't think all of the markup in question in set.mm is recently added. |
I've added some changes to fix underscores in the two last categories:
After this change, there are still two cases not handled by the last
I believe these two cases are issues with Note that some of these changes affect mathboxes (at least @mazsa and @avekens) |
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 all looks good. I approve the changes by @tirix and I think we can merge this now.
If there is something remaining to be done (in set.mm, as opposed to metamath-knife), or we want to fix some of these in a different way, we can always make a follow up pull request.
@tirix metamath-knife is only following metamath-exe here. As the linked HTML pages should show, metamath-exe does not recognize either of these and uses underscores instead of italics, so metamath-knife is right to complain that it won't work. |
+ ` <. A , B >. ` __2 ` = { { { A } , (/) } , { { B } } } ` , justified by
- ` <. A , B >. `_2 ` = { { { A } , (/) } , { { B } } } ` , justified by It is possible to use subscript here, but you can't put a space after the quote. |
Ah, nice. I didn't even realize that was the intention here, but it all makes sense now. Fixed in #3527 |
Makes sense. I've adjusted the set.mm comment for both in #3527 |
See metamath/metamath-knife#135 regarding changing this. |
The problem is that tests are now failing on develop/branches (UPDATE: intermittently, see CACHING NOTE below) because of the new checks in metamath/metamath-knife#127
The diff here fixes two small problems but most of the errors from metamath-knife seem to be from a metamath-knife bug/misfeature/problem as described in more detail at metamath/metamath-knife#128
Because this is blocking all set.mm changes (where "all" is to be interpreted in light of CACHING NOTE below), our choices for the moment would seem to be:
I'd generally pick option 1 based on the general https://outage.party reasoning. But perhaps @tirix and @digama0 have more insight into this change.
CACHING NOTE: some test runs are getting the old metamath-knife, some the new one, see #3522 for more on that.