bugGNU nano - Bugs: bug #50054, nano puts the file buffer in a bad...

 
 

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

bug #50054: nano puts the file buffer in a bad state when first loading a file

Submitter:  David Lawrence Ramsey <dolorous>
Submitted:  Sat 14 Jan 2017 11:01:03 PM UTC
   
 
Severity:  3 - Normal Status:  Wont Fix
Assigned to:  bens Open/Closed:  Closed

Fri 24 Feb 2017 11:49:01 AM UTC, comment #5: 

Since there is no actual observable bad behavior, I'm not changing anything.  (Rather I would remove more settings of edittop from the file reading code -- edittop is a display responsiblity, it should be unrelated to buffer handling.)

Benno Schulenberg <bens>
Group administrator
Mon 16 Jan 2017 08:13:32 PM UTC, comment #4: 

The bad state is that edittop is filebot while current is edittop, and reset_cursor() mishandles that the first time it's called, since its job is not to put edittop in range of current like edit_redraw() or edit_refresh() (both of which should be using the same code to check for current's being offscreen in softwrap mode, but that's a different problem).  When you said you couldn't see how current_y could be less than zero in reset_cursor(), this is how it could be.

Although you're right in that everything seems to be set up properly before update_line() is called, and that, ideally, titlebar() shouldn't have to call reset_cursor().

David Lawrence Ramsey <dolorous>
Group Member
Mon 16 Jan 2017 07:32:35 PM UTC, comment #3: 

Ad 4) The buffer not in a valid state?  That edittop equals filebot is strange, but not invalid: one could reach it in any file by pressing M-\ and then holding down M-+.

Okay, it is not what is intended.  But things are still in flux, getting set up.

Ad 5) Yes, current_y gets calculated to 4 on the first call of reset_cursor().  But upon the second call (before the file gets displayed), it gets calculated to 0, which is correct.

The same for the non-softwrap case: first it becomes -1, then on the second call it becomes 0.

So, as far as I can tell, before any update_line() is called, things are in a good state.  Or do you see any incorrect behavior?

(What I wish would be changed is that titlebar() doesn't call reset_cursor(), because it shouldn't need to.)

Benno Schulenberg <bens>
Group administrator
Sun 15 Jan 2017 12:22:51 AM UTC, comment #2: 

Attached version 2 of the patch.  edittop is also erroneously left set to filebot in do_insertfile(), if you've loaded a file into a new buffer when using multibuffer mode.  The patch fixes this instance as well.

(file #39480)

David Lawrence Ramsey <dolorous>
Group Member
Sat 14 Jan 2017 11:06:32 PM UTC, comment #1: 

The problem seems to originate in open_buffer().  When loading a file into a new buffer, it sets current to fileage and current_x to 0 in order to put them at the top of the file, but leaves edittop unchanged (which means it's set to filebot).  The attached patch fixes this by setting edittop to fileage, as is done with current.

(file #39479)

David Lawrence Ramsey <dolorous>
Group Member
Sat 14 Jan 2017 11:01:03 PM UTC, original submission:  

(Taken from patch #9215.)

Steps to reproduce are as follows:

1. Start in an 80x24 terminal.

2. Use gdb to start nano.

3. Inside gdb, set a breakpoint on reset_cursor(), and then run nano with "--ignore --softwrap reset_cursor-current_y-test.txt". (The file is attached.)

4. The first time reset_cursor() is called will trigger in gdb. This call is actually from a side effect: when the file is first being read in via display_buffer(), load_buffer() calls titlebar(NULL) to display the filename, and titlebar() calls reset_cursor(). However, the buffer is not in a valid state at this point: current is identical to fileage, and edittop is identical to filebot.

5. Trace through reset_cursor() via gdb. There's another problem as well, if you do this again without using --softwrap: since non-softwrap mode uses (current->lineno - edittop->lineno) to get the line number, current_y ends up being -1, but since softwrap mode adds the screen line count from edittop to current[current_x] and goes until it reaches NULL, current_y ends up being 4, which is erroneously in range.

David Lawrence Ramsey <dolorous>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

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 dolorous (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-02-24 bens StatusNone Wont Fix
        Assigned toNone bens
        Open/ClosedOpen Closed
    2017-01-15 dolorous Attached File- Added 0001-tweaks-version-2-fix-file-buffer-s-being-in-a-bad-st.patch, #39480
    2017-01-14 dolorous Attached File- Added 0001-tweaks-fix-file-buffer-s-being-in-a-bad-state-when-l.patch, #39479
    2017-01-14 dolorous Attached File- Added reset_cursor-current_y-test.txt, #39478

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code