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

Formatting bug #93

Open
benjub opened this issue Jul 15, 2022 · 6 comments
Open

Formatting bug #93

benjub opened this issue Jul 15, 2022 · 6 comments

Comments

@benjub
Copy link
Collaborator

benjub commented Jul 15, 2022

As noticed in metamath/set.mm#2689 (comment), it looks like in some cases (but not all) when a $p-statement is written with $p ending a line (hence the next line begins with |-), then the proof is not indented (see currently ~eucalg in set.mm, but notice that e.g. ~stirlinglem5's proof is correctly indented).

@savask
Copy link

savask commented Jul 25, 2022

I think I found the root of the bug. Here's a small example:

  $c A $.
  $( An axiom. $)
  a1 $a A $.
  ${
    $( A theorem.  (Contributed by XX, 01-01-1970.) $)
    th $p
      A $= ( a1 ) A $.
  $}

After save proof */compressed/fast and write source ... /rewrap we get

  $c A $.
  $( An axiom. $)
  a1 $a A $.
  ${
    $( A theorem.  (Contributed by XX, 01-01-1970.) $)
    th $p
      A $=
  ( a1 ) A $.
  $}

The problem seems to be in the getSourceIndentation function of the file mmdata.c. It is used to compute indentation for the proof block, so when it's called in our example the fbPtr pointer points at a newline right after $p token, and this immediately stops the while(1) loop computing indentation.

It seems that the fix is too add the line

  if (fbPtr[0] == '\n') fbPtr--;

right before the while loop. I ran the version of metamath with the fix on set.mm, and the only affected lines are the ones with proofs with broken indentation, so it seems that the fix doesn't do any harm. Formatting is a delicate subject, so I would be glad to hear opinions of other metamath-exe contributors (maybe @digama0 or @wlammen can take a look?).

@wlammen
Copy link
Contributor

wlammen commented Sep 28, 2022

I think one can unconditionally step back one character, so you are on the p of the $p (if this is the keyword), no matter what character is following
The comment /* Go back to first line feed prior to the label */ is misleading. The first line feed (LF) before the keyword determines the indentation. In theory the label and keyword may be separated by one or more LF.

@benjub
Copy link
Collaborator Author

benjub commented Oct 3, 2022

@wlammen or @savask : can you make a PR with the fix you proposed ?

@wlammen
Copy link
Contributor

wlammen commented Oct 3, 2022

I am not going to add a PR on my behalf, before #100 is finished, and that project seems to have ground to a standstill already at the second step.

@savask
Copy link

savask commented Oct 4, 2022

@benjub I have my another PR stuck in the queue (about minimization), so I'll wait till it is resolved.

@benjub
Copy link
Collaborator Author

benjub commented Oct 4, 2022

@savask I think there would be no merge conflicts between the two PRs. I whish I could help (and also for @wlammen's #101) but my C skills are not up to the task. I can understand what your patch in #94 does but cannot certify it is bug-free :(

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

No branches or pull requests

3 participants