bugGNU nano - Bugs: bug #54861, resizing a window doesn't adjust...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #54861: resizing a window doesn't adjust for a relative fill width

Submitted by:  David Lawrence Ramsey <dolorous>
Submitted on:  Wed 17 Oct 2018 11:15:58 PM UTC  
 
Severity:  2 - Minor Status:  Fixed
Assigned to:  David Lawrence Ramsey <dolorous> Open/Closed:  Closed

( Jump to the original submission)

Tue 23 Oct 2018 08:01:12 PM UTC, comment #15: 

Typo: make that "I figured it wouldn't be getting in 3.2".

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Tue 23 Oct 2018 04:04:55 PM UTC, comment #14: 

No problem.

And acknowledged on the justify makeover; I figured it wouldn't be getting in 3.1, at least not the bits that are already in place :)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Tue 23 Oct 2018 02:42:11 PM UTC, comment #13: 

Sorry, yes, I regretted having asked for confirmation, because after an hour I decided I wanted to apply and push out some other patches but wanted to keep the corrective patch right after your commit, the commit to be corrected.  So I applied it, also because I figured that it restored the code to what is had been for a long time, and no one ever complained that --fill didn't override 'set fill', so probably the code was correct.  (And I did a quick test, of course, but that has limited value.)

I'll hold off on the renaming, and I'm holding off on the justify makeover until 3.2 is out.  I'm concentrating on the --zap and linter stuff for that release.

Benno Schulenberg <bens>
Project Administrator
Mon 22 Oct 2018 11:04:22 PM UTC, comment #12: 

Just in case, I've attached an updated version of the patch to swap the names of wrap_at and fill.  It's now against git 71de0b9.

(file #45261)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Mon 22 Oct 2018 06:43:16 PM UTC, comment #11: 

I appreciate that it's in, but if you're going to wait for a confirmation, could you wait for more than only a few hours?  I wasn't in front of my development machine until just a little while ago.

As for swapping the names of fill and wrap_at, should I update that to apply to current git, or should I hold off?  I'm concerned because of its potential breakage of the undoable-justify patch set.  (If I had to choose between the two, I'd prefer to focus on the latter first, since functionality beats variable renames.)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Sun 21 Oct 2018 03:27:23 PM UTC, comment #10: 

I didn't wait for your confirmation but have pushed the change: commit 5c5806bb.  Thanks for reporting.

Benno Schulenberg <bens>
Project Administrator
Sun 21 Oct 2018 10:11:41 AM UTC, comment #9: 

Ooof...  I had compared your patch with the offending commit twice, and twice failed to notice the difference, even though I felt your patch was shorter.  :|  But in fact it lacked the reversal of:

-              ssize_t wrap_at_cpy = wrap_at;
+              ssize_t fill_cpy = fill;

and

-                      wrap_at = wrap_at_cpy;
+                      fill = fill_cpy;

So, to complement your commit 16a3dc90 and complete the revert, for me the attached patch also restores the "--fill overrides 'set fill'" functionality.  Can you confirm?

(file #45253)

Benno Schulenberg <bens>
Project Administrator
Thu 18 Oct 2018 09:41:00 PM UTC, comment #8: 

Patch 0002 in the previous set had an accidentally-added cosmetic rewrapping.  The attached version fixes this.

(file #45240)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Thu 18 Oct 2018 09:20:22 PM UTC, comment #7: 

Argh.  Forget the previous patch set; it didn't set the proper values at a few points in the first patch, and it didn't account for renaming fill in text.c in the second patch.

The attached new patch set (in a zip file) should work.

(file #45239)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Thu 18 Oct 2018 07:46:14 PM UTC, comment #6: 

No problem.  But it turns out that there's now a worse problem caused by the change, that I missed: -r on the command line no longer overrides fill in the nanorc file.  (I use "fill -1" in my nanorc, and "-r 72" on the command line in some cases.)

The attached patch set does the following: the first fixes the aforementioned problem, and the second swaps the fill and wrap_at variable names as you suggest.

(file #45237, file #45238)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Thu 18 Oct 2018 06:52:04 PM UTC, comment #5: 

Thanks.  And thanks for the improved comments.

Applied and pushed, commit 16a3dc90.

(A small nuisance remains: 'wrap_at' corresponds to the value of the --fill option, and 'fill' is the actual wrapping point.  It would be better if the variables were named the other way around.)

Benno Schulenberg <bens>
Project Administrator
Thu 18 Oct 2018 01:26:34 AM UTC, comment #4: 

Thanks for the info.

As for the typo, oops :)  I'm attaching a version of the patch with a fix for that (and with a mention of your reporting the bug, since that seems to just be part of the commit message).

(file #45231)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Wed 17 Oct 2018 11:54:35 PM UTC, comment #3: 

The patch fixes the issue, but "weapping" in commit message needs to be corrected.

Brand Huntsman <brand>
Wed 17 Oct 2018 11:22:15 PM UTC, comment #2: 

Also, the patch should be tagged as "reported by Brand Huntsman", but I'm not certain how to do it.

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Wed 17 Oct 2018 11:21:01 PM UTC, comment #1: 

The problem was introduced by commit e90b7cf.  The wrap_at variable was actually necessary to preserve relative values of fill, but it was removed.

The attached patch, against git 769a0e6, fixes this.  It puts back the code removed by the erroneous commit, in the process reordering the wrap_at and fill variables and adding a comment to make it clearer that wrap_at is necessary.  (It's not a straight revert because of the other changes; should it be?)

(file #45230)

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.
Wed 17 Oct 2018 11:15:58 PM UTC, original submission:  

Taken from Brand Huntsman's report at:
http://lists.gnu.org/archive/html/nano-devel/2018-10/msg00081.html

"When I resize a terminal window to be larger while nano is running, it resizes the titlebar but still hard wraps at the smaller width. It happens in urxvt and xterm.

It appears that the 'fill' global is not being updated after a resize. And the fill option has COLS added to it which might make it harder to change after a resize."

Specifically, the problem only occurs when using a negative fill value (since such a value is relative to screen size).

David Lawrence Ramsey <dolorous>
Project MemberIn charge of this item.

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by brand (Posted a comment)
  • -email is unavailable- added by dolorous
  • -email is unavailable- added by dolorous (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 14 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-11-12 bens Open/ClosedOpen => Closed
    2018-10-22 dolorous Attached File- => Added 0001-tweaks-swap-the-names-of-wrap_at-and-fill.patch, #45261
    2018-10-21 bens Attached File- => Added complete-the-revert.patch, #45253
    2018-10-18 dolorous Attached File- => Added nano-wrap.zip, #45240
    2018-10-18 dolorous Attached File- => Added nano-wrap.zip, #45239
    2018-10-18 dolorous Attached File- => Added 0001-wrapping-properly-set-the-wrapping-point-on-the-comm.patch, #45237
        Attached File- => Added 0002-tweaks-swap-the-names-of-wrap_at-and-fill.patch, #45238
    2018-10-18 bens Severity3 - Normal => 2 - Minor
        StatusNone => Fixed
        Assigned toNone => dolorous
        Summaryresizing a window to a larger width doesn't change relative width when it should => resizing a window doesn't adjust for a relative fill width
    2018-10-18 dolorous Attached File- => Added 0001-wrapping-make-relative-fill-sizes-work-properly-agai.patch, #45231
    2018-10-17 dolorous Attached File- => Added 0001-weapping-make-relative-fill-sizes-work-properly-agai.patch, #45230
        Carbon-Copy- => Added bens

    Back to the top


    Powered by Savane 3.5