urwid wrap text at max 80 characters

[Boruch-Baum]

When using urlscan in a text window wider than 80 characters, the text doesn't wrap. On a wide screen, that becomes difficult to read. Also, since urlscan is often used with email programs such as mutt, there is a user expectation / standard / convention of lines max-ing out at 80 characters.

[firecat53]

Try the var_width branch. Start urlscan with -w <num> or --width <num>. There's a bug that un-centers the display when the footer display (search, number jump, url open, etc) is cleared that I'm having trouble with, but otherwise it should work as you described.

[Boruch-Baum]

On 2020-01-20 13:45, Scott Hansen wrote:

Try the var_width branch. Start urlscan with -w or --width .

It seems to work. Thanks! I've tested it only on a few files, from both inside and outside of mutt, and so far so good.

There's a bug that un-centers the display when the footer display (search, number jump, url open, etc) is cleared that I'm having trouble with, but otherwise it should work as you described.

Actually, I prefer the text not to be centered. To me, on a wide screen, not only does it look strange, but it introduces leading spaces into any clipboard action. Aesthetically, on wide displays, my expectation remains that LTR text should be left justified. I would argue that most people would expect that, but I don't honestly know and haven't taken a poll. Then there are the RTL languages (Hebrew, Arabic, Farsi, etc.) that expect text to be right justified, but it may be that urwid is unicode-aware enough that the 'left' arg means 'logically prior'... OK, I went ahead and tested with sample Hebrew file, and urwid or urlscan seems to be neanderthal when it comes to RTL support, in that it displays characters in reverse order, AND incorrectly justified.

Since I was toying around anyway, attached is a patch that:

1) Removes the left padding, so paragraphs aren't centered;

2) Changes the criteria for inserting new-lines. I found that OTOH, the current code was inserting to many, breaking the flow of paragraphs, and OTOH wasn't inserting enough to demarcate text.

3) Inserts a new-line character after what I suspect is the markup for the html tag 'HR'. This was appearing as a long string of equals signs.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[firecat53]

Thanks for the comments! The patch didn't get attached...I'm assuming github probably doesn't do email patches? In any case, send PR or via email and I'll check it out when I can.

[Boruch-Baum]

Yeah, github also restricted attaching the file via browser interface. Following is a paste of the patch file. Python being python, the indentations of the snippet need adjusting but the snippetss are small and from the line numbers you can see what I did. Also in file urlchooser.py, at lines 675-678, variable first is defined, checked and set, but never used for anything; I suspect it's leftover cruft from old code an can de deleted.

--- /home/boruch/urlchooser.orig.py 2020-01-20 16:42:19.000000000 -0500
+++ /home/boruch/urlchooser.py      2020-01-20 22:36:10.028053457 -0500
@@ -211,7 +211,7 @@
             self.headerwid = None
         self.top = urwid.Frame(listbox, self.headerwid)
         self.pad = int((self.term_width - self.width) / 2)
-        self.top = urwid.Padding(self.top, left=self.pad, right=self.pad)
+        self.top = urwid.Padding(self.top, left=0, right=self.pad)
         if self.urls:
             self.top.base_widget.body.focus_position = \
                 (2 if self.compact is False else 0)
@@ -687,12 +687,19 @@
             if not usedfirst:
                 markup.append(('msgtext:ellipses', '...\n'))
             for chunks in group:
+                if len(chunks) == 0:
+                    markup += '\n\n'
+                else:
                 i = 0
                 while i < len(chunks):
                     chunk = chunks[i]
                     i += 1
                     if chunk.url is None:
+                            if chunk.markup[:2] == "  ":
+                                markup += '\n'
                         markup.append(('msgtext', chunk.markup))
+                            if len(chunk.markup) < 3 or re.search("^=+$", chunk.markup):
+                                markup += '\n'
                     else:
                         if (dedupe is True and chunk.url not in urls) \
                                 or dedupe is False:
@@ -713,7 +720,6 @@
                                    ('urlref:number:braces', ' ['),
                                    ('urlref:number', repr(url_idx)),
                                    ('urlref:number:braces', ']')]
-                markup += '\n'
             if not usedlast:
                 markup += [('msgtext:ellipses', '...\n\n')]
             items.append(urwid.Text(markup))

[firecat53]

Couple of issues:

1. In this chunk:

                    for chunks in group:
   -                   i = 0
   +                   if len(chunks) == 0:
   +                       markup += '\n\n'
   +                   else:
   +                       i = 0

If len(chunks) == 0, i is never initialized and urlscan crashes on the while i .. line.

2. The display problem still exists when the footer is cleared. If you hit C or a number to go to a certain line, when the footer clears, the display width decreases until another key is pressed. This is exactly what was happening with the centered display :/ Are you seeing that behavior as well??

[Boruch-Baum]

On 2020-01-23 20:04, Scott Hansen wrote:

Couple of issues:

1. In this chunk: for chunks in group:

   • i = 0
   • if len(chunks) == 0:
   • markup += '\n\n'
   • else:
   • i = 0

   If len(chunks) == 0, i is never initialized and urlscan crashes on the while i .. line.

If you mean the line while i <len(chunks):, I don't have that; That's a python indentation misunderstanding. I have that whole hunk of code indented along with i = 0.

2. The display problem still exists when the footer is cleared. If
   you hit C or a number to go to a certain line, when the footer
   clears, the display width decreases until another key is
   pressed. This is exactly what was happening with the centered
   display :/ Are you seeing that behavior as well??

No. I can't reproduce what you're describing.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[Boruch-Baum]

Do want more input from me about this?

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[firecat53]

Well...I've been stalled out. The display width change on my machine is baffling me :( I'll keep plugging away it.

[Boruch-Baum]

On 2020-01-30 20:19, Scott Hansen wrote:

Well...I've been stalled out. The display width change on my machine is baffling me :( I'll keep plugging away it.

Do you mean that my code changes aren't working for you? I could submit it as a PR to be certain we're using the exact same code.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[firecat53]

I don't think it's your code. It's the same behavior I noted initially with the centered window that's also occurring with left-justified window. I'll have to record a gif at some point to show you what's happening. Very annoying!! I may just have to try restructuring the urwid widgets...not sure. Haven't been able to sit down with it for a week now...

[firecat53]

Here's what's happening (open urlscan -w80, C to copy to clipboard, then j after screen glitches to get it back to normal.)

https://asciinema.org/a/AZTHfa7nBPDir4n2QBHxSHu8B

It's the same behavior as before your patch:

https://asciinema.org/a/87JMldjcp5NrRQ2mDEs1Ah9Bp

I'm stumped :/

[Boruch-Baum]

On 2020-02-10 18:55, Scott Hansen wrote:

Here's what's happening:

[1]https://asciinema.org/a/AZTHfa7nBPDir4n2QBHxSHu8B

I don't reproduce the behavior illustrated in the gif. When I use the urlscan, the width doesn't ever change. One drawback of using asciinema is that it doesn't visually display your keystrokes, but I presume that you typed 'c' twice, or 'c' followed by return. IMO, a better, simpler and more straight-forward alternative to asciinema would be something like 'ffmpeg' for the recording, along with 'screenkey' for displaying what you type. Both are standard programs, available in most distro repositories, and don't require anything special to playback, just the user's normal player. I didn't enjoy using asciinema.

I'm stumped :/

Have you triple-checked that your own install of urlscan is clean? I'm suggesting that only because, like I said, I'm not reproducing your symptoms.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[firecat53]

I installed the var_width branch on an Ubuntu install (python 3.7) (I'm on Arch, w/ Python 3.8) and I'm still seeing the darn width changes when the footer is triggered (via copy to clipboard or selecting an entry number). Happens with or without your patch.

Your patch also puts huge amounts of white space into the urlscan display on a good number of the emails I tested it with. Would you be able to send a screenshot or screencast of the white space differences with and without your patch? I'm not seeing the advantage :-)

Thanks!

[Boruch-Baum]

Attached are two sample screenshots, but only with my patch; I don't want to mess with playing with versions just now. I get no change to the display when pressing 'C' or pressing a number, except for a red line appearing at the bottom of the window with a notification of the action performed.

Here's an idea: compose a small simple html file that illustrates your bug, send it to me, and I can send you another screenshot, if you think that will help. Also, I just now did a diff between our versions and I see that you've been doing a lot of other work on the file, but the bottom line is that the main complaint you have is something that you say occurs even without my patch.

I do have a modification to my patch, as follows (line ~212):

        self.top = urwid.Frame(listbox, self.headerwid)
        self.pad = int((self.term_width - self.width))
        self.top = urwid.Padding(self.top, left=0, right=self.pad)

That might be related to your complaint about the extra whitespace, and might have solved it.

[firecat53]

So, mind if I ask what OS, python version and terminal you are using?

[firecat53]

AHA! Solved it finally!! I was using a thread for the timeout of the footer notification and it was just blind luck that it worked. Urwid is not thread safe. That was what was causing the display troubles. Switched to using self.loop.set_alarm_in and it works perfectly!

Now I can take a look at your patch and the modification.

[firecat53]

I've changed the code to left justify instead of center.

Either I've entered the remainder of your patch incorrectly (for the white space fixes), or I'm not seeing the benefit. There's almost no difference for the email from Github, and for the email from Mint it becomes basically unreadable because of the excess white space entered. I've checked it with other emails and it's about the same: either very little difference or a huge amount of extra whitespace is added.

Here's the patch I'm using:

diff --git a/urlscan/urlchoose.py b/urlscan/urlchoose.py
index f8b3505..2c81db0 100644
--- a/urlscan/urlchoose.py
+++ b/urlscan/urlchoose.py
@@ -677,12 +677,19 @@ class URLChooser:
             if not usedfirst:
                 markup.append(('msgtext:ellipses', '...\n'))
             for chunks in group:
+                if len(chunks) == 0:
+                    markup += '\n\n'
                 i = 0
                 while i < len(chunks):
                     chunk = chunks[i]
                     i += 1
                     if chunk.url is None:
+                        if chunk.markup[:2] == "  ":
+                            markup += '\n'
                         markup.append(('msgtext', chunk.markup))
+                        if len(chunk.markup) < 3 or re.search("^=+$", chunk.markup):
+                            markup += '\n'
+
                     else:
                         if (dedupe is True and chunk.url not in urls) \
                                 or dedupe is False:
@@ -703,7 +710,6 @@ class URLChooser:
                                    ('urlref:number:braces', ' ['),
                                    ('urlref:number', repr(url_idx)),
                                    ('urlref:number:braces', ']')]
-                markup += '\n'
             if not usedlast:
                 markup += [('msgtext:ellipses', '...\n\n')]
             items.append(urwid.Text(markup))

[firecat53]

I merged the text width changes. If you want to continue figuring out the whitespace issues, please submit a separate PR so we can work from the same code. Thanks!

[Boruch-Baum]

OK. BTW, I normally use a text browser, so I didn't realize you had posted screenshots a few days ago. From looking at them, it seems that either the patch isn't applied, or maybe you didn't invoke the feature by using the new parameter -w 80, because there is no word-wrap happening in the first screenshot, labeled "with patch".

[firecat53]

'With patch' referred to your whitespace patch only. I just wasn't using the -w parameter for the screenshots.