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

Wrap text to column limit. #4

Closed
swsnr opened this issue Jan 7, 2018 · 12 comments · Fixed by #188
Closed

Wrap text to column limit. #4

swsnr opened this issue Jan 7, 2018 · 12 comments · Fixed by #188
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@swsnr
Copy link
Owner

swsnr commented Jan 7, 2018

Use textwrap, perhaps, to wrap all content to the column size of the TTY.

Will perhaps be tricky with termion formatting characters.

@swsnr swsnr added the help wanted Extra attention is needed label Jan 16, 2018
@swsnr
Copy link
Owner Author

swsnr commented Aug 26, 2018

We can't directly wrap text (eg, w/ textwrap), because we need to account for formatting escapes and for indentation.

But we can use unicode_width to compute the length of text as we write it, and then keep track of the current column being written to. If writing would exceed the column limit we can scan backwards for the first whitespace before column limit, wrap the text and try again until the entire text is written.

@swsnr swsnr self-assigned this Sep 7, 2018
@swsnr swsnr added enhancement New feature or request and removed enhancement New feature or request labels Jan 6, 2019
@swsnr swsnr removed their assignment Jan 6, 2019
@swsnr swsnr pinned this issue Nov 27, 2019
@swsnr swsnr unpinned this issue Dec 22, 2019
@agschaid
Copy link

That would be a great to have. E.g. in cases where links get transferred to the bottom of the document the shorter lines are very disturbing.

Just give me 3 or 4 months to learn a little Rust (I really want that feature ;) )

@igalic
Copy link
Contributor

igalic commented Sep 15, 2020

consolemd does, indeed uses the python equivalent of textwrap: kneufeld/consolemd@6a2c6ee
which means it'll occasionally go under the provided wrap limit, but, eh it's good enough as a start

@swsnr
Copy link
Owner Author

swsnr commented Sep 15, 2020

@agschaid Take all the time you need; I don't think I'll fix this anytime soon.

@igalic I don't think that's good enough for me. It ought to be done right.

@igalic
Copy link
Contributor

igalic commented Sep 15, 2020

i understand, @lunaryorn!

i suppose the rust way would be to create a wrapper type that can represent the output on the console, but can also be used as a String or &str for feeding into textwrap

@orasunis
Copy link

orasunis commented Sep 18, 2020

I believe there is no need to create a wrapper type as textwrap supports ANSI escape codes for a few months now (mgeisler/textwrap#179).
I don't know how well that works with windows though.

@swsnr
Copy link
Owner Author

swsnr commented Sep 18, 2020

@orasunis I guess that'd be a good start. I haven't really looked at it, but if it handles colours, OSC 8 hyperlinks and iterm marks well, it might even be a perfect solution 🙂

@agschaid
Copy link

just to put my "commitment" into perspective: I have two kids. So my "3 or 4 months" can quickly grow into "2 or 3 years" ;)

I think this is a good project/motivation for me to get a little into rust. But don't count on me.

@swsnr
Copy link
Owner Author

swsnr commented Sep 21, 2020

@agschaid There's no commitment here. We all do this in our free time, we all have a life and priorities 🙂

@swsnr swsnr self-assigned this Oct 18, 2020
@mgeisler
Copy link

mgeisler commented Dec 3, 2020

Hi all, I saw a link to this on mgeisler/textwrap#179 -- just wanted to say that textwrap will indeed ignore all ANSI escape sequences since version 0.12. Ignoring means "they don't contribute to the string width", so the wrapping computations are not affected by the escape characters any longer.

Will perhaps be tricky with termion formatting characters.

I've actually been playing around with this recently and I wrote a little demo program: https://github.com/mgeisler/textwrap/blob/master/examples/interactive.rs

It uses termion and if you modify it to use colored text, then you'll see that you can indeed very easily run into problems. Basically, textwrap::wrap will give you back a Vec of strings, complete with the original escape codes. If you just print those to the terminal everything works fine. However, my example program draws a red border around the text and so I use code like

        write!(
            stdout,
            "{}{}│{}",
            cursor::Goto(col - 1, row),
            color::Fg(color::Red),
            color::Fg(color::Reset),
        )?;

This is a problem if there is, say, blue text which was supposed to be wrapped over two lines: the color now stops at the point of the color::Fg(color::Reset) code.

Perhaps you don't have such borders and then things are easy...?

@swsnr
Copy link
Owner Author

swsnr commented Dec 3, 2020

ANSI sequences aren't the issue; the issue is more that pulldown is a pull parser, so we don't get the text at once but rather scattered over many different events.

@mgeisler
Copy link

mgeisler commented Dec 3, 2020

Hi @lunaryorn,

I just realized that we talked about stateful wrapping over in mgeisler/textwrap#224 :-)

So yeah, if you get your text one piece at a time, then it'll be harder to use textwrap. I "cheated" and simply accumulated the text in mgeisler/textwrap#140 (comment), but I understand that you have a different architecture in mdcat.

With mgeisler/textwrap#234, I'm introducing a new more advanced wrapping algorithm which select optimal break points for an entire paragraph at a time — "optimal" according to some penalties which discourage short lines. This is by its nature also quite stateful. I will see if I can make the original wrapping algorithm work in an incremental fashion again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants