Macaulay2 / M2

The primary source code repository for Macaulay2, a system for computing in commutative algebra, algebraic geometry and related fields.
https://macaulay2.com
347 stars 231 forks source link

line wrapping of documentation is screwed up #1598

Closed DanGrayson closed 3 years ago

DanGrayson commented 3 years ago

Look at this:

Screen Shot 2020-11-18 at 6 10 07 AM

Maybe @mahrud , @d-torrance can help.

d-torrance commented 3 years ago

The problem appears to be with nested lists, e.g.:

i1 : debug Core

i2 : UL LI {"foo", UL LI ("f" | concatenate apply(100, i -> "o"))}

o2 =   * foo  * foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
                ooooooooooooooooooooooooooooooooooo
         ooo

o2 : UL

We're assuming that we only need to indent 4 spaces, but that's only okay for the top level list. https://github.com/Macaulay2/M2/blob/2c9e746102cd4d91c114887eaf10723e0642e5e2/M2/Macaulay2/m2/format.m2#L177-L182

d-torrance commented 3 years ago

Never mind my previous comment -- the indentation is just fine. printWidth will decrease appropriately thanks to recursion. For example:

i1 : debug Core

i2 : UL UL UL UL UL UL UL apply(2, i -> "f" | concatenate apply(100, i -> "o"))

o2 =   *   *   *   *   *   *   * fooooooooooooooooooooooooooooooooooooooooooooo
                                 oooooooooooooooooooooooooooooooooooooooooooooo
                                 ooooooooo
                               * fooooooooooooooooooooooooooooooooooooooooooooo
                                 oooooooooooooooooooooooooooooooooooooooooooooo
                                 ooooooooo

The problem is mixing non-UL's and UL's together in the same LI object. They're getting horizontally joined, but they should probably be stacked instead.

i3 : LI {"foo", UL "bar"}

o3 = foo  * bar
DanGrayson commented 3 years ago

This bug was introduced recently:

Screen Shot 2020-11-18 at 9 32 02 AM
DanGrayson commented 3 years ago

Maybe commit c7f4e4c9f0beeb208f779cc374266cda26f48f3b did it:

commit c7f4e4c9f0beeb208f779cc374266cda26f48f3b
Author: Mahrud Sayrafi <mahrud@berkeley.edu>
Date:   Fri Oct 9 18:18:22 2020 -0500

    change LI to HypertextParagraph
diff --git a/M2/Macaulay2/m2/hypertext.m2 b/M2/Macaulay2/m2/hypertext.m2
index f52d683b6..75d3dcf02 100644
--- a/M2/Macaulay2/m2/hypertext.m2
+++ b/M2/Macaulay2/m2/hypertext.m2
@@ -131,10 +131,10 @@ SUB        = new MarkUpType of Hypertext
 SUP        = new MarkUpType of Hypertext
 TT         = new MarkUpType of Hypertext

--- Lists (TODO: OL)
+-- Lists
 OL         = new MarkUpType of HypertextContainer
 UL         = new MarkUpType of HypertextContainer
-LI         = new MarkUpType of HypertextContainer
+LI         = new MarkUpType of HypertextParagraph
 DL         = new MarkUpType of HypertextContainer
 DT         = new MarkUpType of HypertextParagraph
 DD         = new MarkUpType of HypertextParagraph

Mahrud, what do you think?

@mahrud

d-torrance commented 3 years ago

I think that's exactly what happened -- LI just horizontally joins all its elements now.

I have a working fix -- just writing up the commit message before I submit a PR.

mahrud commented 3 years ago

Interesting. I think LI should be a HypertextParagraph, maybe we can discuss the precise differences between HypertextParagraph and HypertextContainer, or maybe the better fix would be to wrap the individual elements of LI?

DanGrayson commented 3 years ago

Why do you think that?

mahrud commented 3 years ago

Notice the indentation now:

i2 : html UL{LI{"a", SPAN{"b"}},LI{"c"}}

o2 = <ul>
       <li>a<span>b</span></li>
       <li>c</li>
     </ul>

vs in 1.16:

i2 : html UL{LI{"a", SPAN{"b"}},LI{"c"}}

o2 = <ul>
       <li>
     a<span>b</span>  </li>
       <li>
     c  </li>
     </ul>

The way I think about Paragraph vs Container is this: is the item closer in semantics to <div> or <p>. This example demonstrates the main difference:

<div>
  <p>...</p>
</div>

Of course, you can put pretty much whatever tags inside anything you want, but most often UL is a container that contains a bunch of LI items, each as a paragraph.

That's why I think the wrapping code for paragraph should be fixed instead, but I don't have time to fix it, so if you'd rather just revert that commit, go for it.

DanGrayson commented 3 years ago

You have a different view of it than I do. I think the point is that a Paragraph is low-level, and cannot contain things like lists (UL), but a Container can. Since a list item (LI) can contain lists, it can't be a paragraph (P), and the paragraph wrapping code should not be changed.