bugGNU nano - Bugs: bug #52523, undoing text added to the...

 
 

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

bug #52523: undoing text added to the magicline doesn't delete the new magicline

Submitter:  David Lawrence Ramsey <dolorous>
Submitted:  Sun 26 Nov 2017 10:42:24 PM UTC
   
 
Severity:  3 - Normal Status:  Fixed
Assigned to:  dolorous Open/Closed:  Closed

Jump to the original submission

Wed 03 Jan 2018 02:28:57 PM UTC, comment #12: 

Released in 2.9.2.  Thanks for reporting.

Benno Schulenberg <bens>
Group administrator
Thu 30 Nov 2017 08:24:42 PM UTC, comment #11: 

Thanks.  But... the magicline creation should not be moved inside a NANO_TINY thing -- I think.  So... after updating that, and after updating the final comment in the patch to the new situation, pushed: b9638cb7.

Benno Schulenberg <bens>
Group administrator
Tue 28 Nov 2017 11:27:27 PM UTC, comment #10: 

Okay.  Attached updated version that doesn't use the global.

(file #42513)

David Lawrence Ramsey <dolorous>
Group Member
Tue 28 Nov 2017 06:59:29 PM UTC, comment #9: 

I don't like the solution with another global variable.  :|  It should be possible to correctly detect the addition of magic line locally.

Hmmm...  I propose that in do_output() the new magic line should be added only after add_undo() has been called.  (Which makes more sense anyway: first do all the insertion stuff, and then, if needed, create a new magic line.)  When things are that way, it becomes dead simple to detect the imminent addition of a new magic line: openfile->current == openfile->filebot && openfile->current_x == 0.

Benno Schulenberg <bens>
Group administrator
Mon 27 Nov 2017 09:29:33 PM UTC, comment #8: 

Okay.  However, after more testing, the last patch doesn't always detect the magicline properly.  With it applied, run:

./nano --ignore

Then press Enter to add a second line, Up to move back to the first line, and "x" to add "x" to the beginning of the first line.  Then press Meta-U: both the "x" and the second line will be gone.

There's a problem detecting when the magicline is added, which is currently done after the fact: there's no way to tell whether the blank line after the current line was manually added or whether it's the magicline.

The attached patch fixes this: it introduces a global boolean that's set to TRUE only when the last text added via output_text() produced a new magicline, which is then used to properly detect when a magicline is added.  (A global is necessary because the undo flags are local to text.c, while output_text() is in nano.c.  It still should be relatively simple, though.)

(file #42504)

David Lawrence Ramsey <dolorous>
Group Member
Mon 27 Nov 2017 08:13:35 PM UTC, comment #7: 

Oh, but nano not removing the added magicline but clearing the "Modified" flag and allowing the user to exit immediately (without being asked whether to save), really isn't a nasty bug.  Because for all intents and purposes (of the user) the file was unmodified, so it is perfectly fine to exit without saving.

Benno Schulenberg <bens>
Group administrator
Mon 27 Nov 2017 08:04:43 PM UTC, comment #6: 

David: Thanks for the patch.  Looks good.  Will apply somewhere in the coming days.

Peter: Both Nano in its default setup and Pico have always modified the file when it doesn't end in a newline: when you open the file and save it, it will have a newline added.  You are mostly right that this is not a recommendation for an editor, but... it has an advantage as well: it stops diff from complaining that there is no newline at end of file.  And since I do a lot of diffs, I have removed the "nonewlines" option from my nanorc, since half a year or so.

Benno Schulenberg <bens>
Group administrator
Mon 27 Nov 2017 05:36:10 PM UTC, comment #5: 

Peter: Yes, automatic modification is bad, and it shouldn't be difficult to fix.

Benno: Forget the previous patch; it handled undo but not redo (and the latter is a simple two-line addition).  The attached patch handles them both properly.

(file #42502)

David Lawrence Ramsey <dolorous>
Group Member
Mon 27 Nov 2017 05:33:59 PM UTC, comment #4: 

Hmm I see what you mean, that is a nasty bug. Because it just allowed me to close no questions asked, I assumed the file was unchanged. But if you save it after undo, it has the CR.

Personally I would never use an editor in such a mode where it automatically modifies a file, but when it does, it should be recorded. I would consider this very damaging to the reputation of an editor. But... it shouldn't be too difficult to set the DIRTY flag in this case, right?

Peter Passchier <pepa65>
Mon 27 Nov 2017 05:23:03 PM UTC, comment #3: 

Peter: I've tested it with c17a8a9, and if you toggle line numbering mode, you can see the magicline added by the text is still there.

Benno: Considering that magiclines are properly handled by other undo operations such as Backspace, I don't see why this wouldn't be a problem.  In any case, I have a simple patch for this now (against git c17a8a9, and attached).  It requires adding another undo flag to indicate that there was a magicline before, but that's unavoidable.

(file #42501)

David Lawrence Ramsey <dolorous>
Group Member
Mon 27 Nov 2017 04:55:44 PM UTC, comment #2: 

Following the recipe with c17a8a98364d7258455227e9927af286ebc557dc it now seems to work, the magicline is removed when doing Undo.

Peter Passchier <pepa65>
Mon 27 Nov 2017 03:27:18 PM UTC, comment #1: 

True... if you "undo" text added to the magicline, the extra magicline that was produced by adding the text is then not deleted.  But... I cannot be bothered about it.  If you have a simple patch for this, fine.  But if not... it are harmless newlines at the end of a file.

Benno Schulenberg <bens>
Group administrator
Sun 26 Nov 2017 10:42:24 PM UTC, original submission:  

This can be reproduced as follows:

1. With nano git 20aa167, run:

./nano --ignore

to ensure that the magicline is used.

2. Press "x".  An "x" will be added to the magicline, creating a new magicline below.

3. Press Meta-U to undo the text addition.  The "x" will disappear, but the new magicline below where the "x" was will still be there, so the text is thus not restored exactly to its previous state, as it should be when using undo.

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 pepa65 (Posted a comment)
  • -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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-01-03 bens Open/ClosedOpen Closed
    2017-11-30 bens StatusIn Progress Fixed
    2017-11-28 dolorous Attached File- Added 0001-undo-if-added-text-adds-a-magicline-make-undo-redo-h.patch, #42513
    2017-11-27 dolorous Attached File- Added 0001-undo-if-added-text-adds-a-magicline-make-undo-redo-h.patch, #42504
    2017-11-27 bens StatusNone In Progress
        Assigned toNone dolorous
    2017-11-27 dolorous Attached File- Added 0001-undo-if-added-text-adds-a-magicline-make-undo-redo-h.patch, #42502
    2017-11-27 dolorous Attached File- Added 0001-undo-if-added-text-adds-a-magicline-make-undo-remove.patch, #42501
    2017-11-27 bens Summaryundoing text added to the magicline doesn't put the old magicline back undoing text added to the magicline doesn't delete the new magicline

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code