-
Notifications
You must be signed in to change notification settings - Fork 26
[bug] Text wrapping: broken list case; 2x indent; length overflow #82
Comments
Thanks again for your helpful issues and uncovering the cursed fruit of my work !
is stable and pretty good looking, i don't see a problem with it. The list && enum thing is bad tho |
I think you've missed some of my points, specifically:
These are the main problems right now with version |
I indeed missed that first point! I might still be missing the second one tbh |
I can't reproduce |
I've left tested version, which is indeed an old version right now. I'll update and test again. |
This is default behavior on #very-long-long-long-long-long-function-name[Lorem ipsum dolor sit amet, consectetur adipiscing
elit, sed do eiusmod tempor incididunt ut labore
et dolore magnam aliquam quaerat voluptatem. Ut
enim aeque doleamus animo, cum corpore dolemus,
fieri.] As I already said, it should be something like this:
Because next formatting style will (could) introduce a space at the beginning: #very-long-long-long-long-long-function-name[// Here an (un)wanted whitespace will appear.
Lorem ipsum dolor sit amet, consectetur // Even if this line is not indented.
adipiscing elit, sed do eiusmod tempor
incididunt ut labore et dolore magnam aliquam
quaerat voluptatem. Ut enim aeque doleamus
animo, cum corpore dolemus, fieri.] // (1)
] // (2) I prefer |
I finally get it, you would want me to modify the ast in that manner! I don't know How I feel about that ﹀ not breaking here cause that modifies the content which is forbidden
#very-long-long-long-long-long-function-name[Lorem // breaking here cause it's over max length
ipsum dolor sit amet, consectetur adipiscing
elit, sed do eiusmod tempor incididunt ut labore
et dolore magnam aliquam quaerat voluptatem. Ut
enim aeque doleamus animo, cum corpore dolemus,
fieri.] |
But that's not possible cause the child is not aware of the parent, what you propose is sensible, I'll think about it |
I fixed the other one btw |
I've just written a big-ish comment, and you've replied to my previous comments (probably). So you have to be more specific about "fixed the other one". I don't know what you are referring to. Update: I've checked new commits. You were talking about list issue. I'll check it out. |
Sorry XD |
All lists except terms are now working (in simple cases, I haven't tried anything complex). |
I also fixed
becoming
|
Let's look at terms now |
Your latest #56fbba6 broke your "1 indent" fix. Now it again adds 2 indents but in one run! This is progress with one step back. |
I totally forgot about terms, right now they're a bit broken If I try to apply the same rules as enums and list for some unknown reason:tm:
I don't know what you're talking about |
Last commit again introduced the initial issue: #f[Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.] #f[Lorem ipsum dolor sit amet, consectetur adipiscing
elit, sed do eiusmod tempor incididunt ut labore
et dolore magnam aliquam quaerat voluptatem. Ut
enim aeque doleamus animo, cum corpore dolemus,
fieri.] 2 indents (in 1 run) instead of 1. |
I think there's one level of indent for the func call and one for the content block, wouldn't you say it makes sense?
would you say this deserves a special case and it should only be one level of indent? |
I thought TermItems were broken but I discovered a bug! (I was stuck on it for a while), I'll open another issue and ping you |
Yes, that would make sense, if
Since If // If was written in one line
#f({ sym.RR })
// If was written differently
#f({
sym.RR
}) |
Content blocs are space sensitive but not indent sensitive 😎 |
It's literally the same thing, nothing is changed. I don't understand what this can prove. Also, please use inline code formatting in Markdown, it improves readability of your examples. |
it shows as is but the left one has 4 spaces total and the right one has 2 spaces total, I don't know why it doesn't show on github |
Because Markdown is the same as LaTeX/Typst, it ignores multiple whitespaces no matter how many there are of them. |
My point exactly then, are you convinced? knowing this which do you think is the prettiest, one or two indents in this case? |
It's not indent sensitive because you already introduced the first whitespace by inserting a newline character: a#f[some content]b
a#f[
some content]b These 2 already will produce different output, but after formatting: a #f[some content]b
a #f[
some content
]b They will output the same output, but not the one that is desired. In the first case, it adds a new whitespace (no one asked), which is very bad! We need to make a new issue for this one. In the second case, it, again, adds a new whitespace before the function call, then a This is a pretty valid use case and the formatter alters the output, which should be fixed right away. |
the program doesn't break the line in the cases you mentioned, it only modifies the space if it's there
|
"Doesn't break the line"? Which line are you talking about? The whole #82 (comment) was about introduction of an additional whitespace if there is some text next to a function call. Which supposed to counter your point in #82 (comment). Look at my examples in #82 (comment) more carefully and check the output in PDF. |
To me
becomes
I didn't understand you were talking about that space, i thought you were talking about a linebreak elsewhere, indeed that's a problem yea, although not hard to fix |
Now, that you understood what I was talking about, think again about newline after
These 2 will produce different output: a#f(
[..]
)b
a#f(
[
..
]
)b And this is exactly why |
this
never becomes that
so I fail to see the issue it might become that
|
the problem I see is this
becoming that
|
Exactly. See #84. |
Yes so it's not about the two level of indentation!!? |
You contradict yourself in #82 (comment). Maybe you should rewrite a more accurate example of what you meant, because right now your thoughts are disconnected from your examples (pseudocode).
It's both. Now that there is a separate #84 issue, we are left with the double indentation problem. |
there is no double indentation problem, double indentation has no influence on the final result. |
I am not contradiction myself cause
is not
|
I'll write an example in a few! |
Ok, I should've used double indentation discussion/choice then. Of course, the level of indentation doesn't change the output, this we can agree with. |
In #82 (comment) you've written this:
In #82 (comment) you've written this:
First, you show why there is a 2 times indent with [
..
] This is a contradiction. If this doesn't happen or won't happen, then where do you get the 2nd indentation? Show me a more accurate example, if your opinion on double indentation still stands. |
Let me repeat filling the ..
might format as this
but this never will, that's why there is no problem with 1 indent, 2 indent etc
|
Ok, now I understand you too!)) This is a tough one, because the minimalist in me wants to optimize the code by removing redundant There is also a question of length. Do we really need to break 1 line into 3+? If the one-liner does not exceed the max line length, then it should stay unformatted. So, here is what I think: before formatting// length <= max:
#f([content])
#f[content]
#f([ content ])
#f[ content ]
// --------------
// length > max:
#f([Lorem ipsum dolor sit amet, consectetur adipiscing]) // Example with 4 potential versions of formatting.
#f[Lorem ipsum dolor sit amet, consectetur adipiscing] // Example with 4 potential versions of formatting.
#f([ Lorem ipsum dolor sit amet, consectetur adipiscing ])
#f[ Lorem ipsum dolor sit amet, consectetur adipiscing ]
// ------------- after formatting// length <= max:
#f[content]
#f[content]
#f[ content ]
#f[ content ]
// --------------
// length > max:
// The next 2 examples should be formatted the same way, but which one is better?
// style 1
#f[Lorem ipsum dolor sit amet, consectetur
adipiscing]
// style 2
#f[Lorem ipsum dolor sit amet, consectetur
adipiscing]
// style 3
#f(
[Lorem ipsum dolor sit amet, consectetur
adipiscing]
)
// style 4
#f(
[Lorem ipsum dolor sit amet, consectetur
adipiscing]
)
#f[
Lorem ipsum dolor sit amet, consectetur
adipiscing
]
#f[
Lorem ipsum dolor sit amet, consectetur
adipiscing
]
// ------------- Notes:
|
I hear you, but I'm wondering, what do we do with this then?
this? (like right now)
It messes with the ast very much and I need to think if that's really okay or not, I'm a slow woman, I like to take my time ^^' I'll also ask in the discord to gather opinion on if this is ok I'm also thinking, if i was a beginner and I saw some parenthesis go away, that would be weird (whereas, a warning that the parenthesis are not needed would be nice) |
Look, this again (2 args) is not what we were discussing at first, and it's not even an issue in this issue (#82). Don't make things more complicated, you should avoid doing that. For the 2 argument question, you probably should open a new issue, or we can discuss it elsewhere.
If you agree with my formatting (in #82 (comment)) about 1 argument The last thing is an accurate line length checking, so that lines with text don't exceed the configured limit. P.S. I don't know how hard/easy these new feature implementations are, but you are doing a great job! I don't think I've ever seen someone implementing new features/fixing bugs left and right just like that *snap*. It feels so cool to be able to open a feature/bug issue, and it will be done in the same day or maybe a couple of days. Unbelievable. |
There are too many things in this issue, let's open a discussion about |
Thanks a lot for the praise 😊 😊 ❤️ |
One other thing that helps is that this project is of limited complexity, it's only 1626 LoC right now, I mean, it's a good bit already and I'm v proud but it's v humanly comprehensible |
Done: #86.
I WISH!!! I just currently don't have time, and didn't have time for a while. Learning Rust (and Go) is my priority, when I'll have more free time.
When I'll reach level "Beginner" in Rust, I'll definitely would like to contribute the code itself. But everything could change, so no promises. ;) |
This is a separate issue, related to the #74, since the text wrapping itself kinda works. Now it's time for bugs!
Before:
We have next things: - thing 1; - thing 2; - thing 3. #f[Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.]
With default:
And version:
0.2.0#578fabc
After 1st run:
We have next things: - thing 1; - thing 2; - thing 3. #f[Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.]
After 2nd run:
We have next things: - thing 1; - thing 2; - thing 3. #f[Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.]
After 3rd run:
We have next things: - thing 1; - thing 2; - thing 3. #f[Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.]
After 4th run:
We have next things: - thing 1; - thing 2; - thing 3. #f[Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.]
Next run will be as in 3rd, after as in 4th and so on.
But with
max_line_length = 80
it stops formatting after 3rd run.Note that the text lines exceed the
50
mark, which is also wrong (same with80
).What's wrong:
list
lines with non-list
lines if they are on adjacent lines (it could be/
/-
/+
/\d+\.
)0.2.1#1817538
version-
/+
/\d+\.
are fixed/
(terms) also got fixedit takes 2 runs to add 2 indents to the content block with a text (which is a function argument) — fixed at least in0.2.1#1817538
content
block with a text (which is a function argument)How it supposed to look like:
We have next things: - thing 1; - thing 2; - thing 3. #f[Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.]
But since the function name could be very long, it should format like this (either only for long function names or for all cases):
We have next things: - thing 1; - thing 2; - thing 3. #very-long-long-long-long-long-function-name( [Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim aeque doleamus animo, cum corpore dolemus, fieri.] )
This last one can be debatable, I assume, but the main problem, is that Typst right now can't do something like this (shell):
Typst instead of one additional space will add a new paragraph if
\
is used right after[
(same with]
). So, to line up all thecontent
block lines, we can use otherwise redundant()
to wrap thecontent
block.Note that if the function name is very long, it could exceed the max line limit, but we can't do anything about it (we can't cut it in half).
The text was updated successfully, but these errors were encountered: