bugGNU nano - Bugs: bug #46900, justifying should be covered by...

 
 

bug #46900: justifying should be covered by the general undo system

Submitted by:  Benno Schulenberg <bens>
Submitted on:  Thu 14 Jan 2016 04:44:29 PM UTC  
Votes:  3  
 
Severity:  3 - Normal Status:  None
Assigned to:  None Open/Closed:  Open

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Thu 09 Aug 2018 01:20:58 AM UTC, comment #11:

I'll get back to this as soon as I can. (Hooking up the alternate spell checker to undo/redo involves some of the same issues that hooking up justify to undo/redo will have to deal with, but the former is simpler than the former because it already works on its own buffer instead of within the file, which is why I went with it first. Also, some of the ways COUPLE_BEGIN/COUPLE_END have been extended for alternate spell-check will be used by justify as well, so some of the work is already done.)

After some thought, the proposed approach of having a function that's called once by ^J and multiple times by M-J may not be as feasible as it sounds, given their different requirements of how much of the file to back up. Justifying multiple non-contiguous paragraphs while preserving the text between them is the main problem that this approach doesn't handle well. So, instead...

What about a function that takes a buffer, and then scans through and justifies all paragraphs in that buffer, leaving non-paragraph parts of that buffer untouched? The main differences between justify modes would then be as follows:

^J: Scan for next paragraph in the file. Put that paragraph in the buffer. The function scans for and justifies all paragraphs in the buffer: only that one paragraph is found and justified, as expected.

M-J: Put the entire file in the buffer. The function scans for and justifies all paragraphs in the buffer: all paragraphs in the file are found and justified, as expected.

Hypothetical ^J with mark on: Scan for next paragraph within the marked text. Put that paragraph in the buffer. The function scans for and justifies all paragraphs in the buffer: only that one paragraph within the marked text is found and justified, as expected.

Hypothetical M-J with mark on: Put all the marked text in the buffer. The function scans for and justifies all paragraphs in the buffer: all paragraphs within the marked text are found and justified, as expected.

David Lawrence Ramsey <dolorous>
Project Administrator
Sat 28 Jul 2018 09:42:42 PM UTC, comment #10:

I wonder whether the justify code as it is needs to be restructured first in order to make this easier. As things are, the first patch in the zip file (the one to remove redundant checks for having justified something) could probably go into the current code after sufficient testing.

David Lawrence Ramsey <dolorous>
Project Administrator
Sat 28 Jul 2018 09:20:45 PM UTC, comment #9:

Started trying to make justify work in its own buffer. It's still unfortunately broken in some ways, which are documented in the commit messages.

It's against the same git version, and is now a sequence of 4 patches in the attached zip file.

(file #44648)

David Lawrence Ramsey <dolorous>
Project Administrator
Fri 27 Jul 2018 05:24:05 AM UTC, comment #8:

Attached an improved rough attempt, still against the same git version. It now tweaks comments only when necessary, and it's been split into two patches.

The first patch removes the redundant checks for having justified something (the simpler task), and the second patch splits out the justification function for justify/full-justify (the more complex task). The problems it causes are unchanged so far, unfortunately.

(file #44633, file #44634)

David Lawrence Ramsey <dolorous>
Project Administrator
Thu 26 Jul 2018 04:56:19 AM UTC, comment #7:

Attached first, very rough attempt at splitting out the justification function for justify/full-justify: against git f82eefa.

It tweaks a few comments, but doesn't adjust indentation yet. I have no idea how useful you might find it, but I figure it's a start.

The major problem with it is in the backup_lines() call: doing a full-justify seems broken, but if you comment out the backup_lines() call, you can see that the attempt to save the original lines for unjustifying is the real source of the problem. It's documented more in the commit message for now.

(file #44623)

David Lawrence Ramsey <dolorous>
Project Administrator
Tue 24 Jul 2018 11:43:15 PM UTC, comment #6:

Not yet. I've been busy the past few days, and when I'm not, I'm still studying the justify code to get back up to speed on it. It's been ages since I looked at it in much detail. [Not to mention, I've occasionally gotten distracted by other, much easier to squish bugs :).] Also, the COUPLE_BEGIN and COUPLE_END undo routines aren't that well documented; I know it's several undos/redos at one time, but a few more comments would be helpful to indicate what's being undone/redone, without having to keep digging into the git history.

Although if you're looking at it too, that should be a big help. Pulling ^J and M-J out of each other is a good first step (since revamping them to work on a passed-in buffer rather than the current file buffer is needed, but much more difficult, so we really should take the easier steps first), and putting it in a branch is definitely the right approach. I don't know whether I still have commit access, but I can certainly send patches.

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 23 Jul 2018 07:10:03 PM UTC, comment #5:

David? Have you started with this? I ask because I'm looking at this too. I would first want to pull ^J and M-J out of each other: to extract a function that is called once by ^J and called in a loop until reaching filebot by M-J.

Can we do this in a branch that I push to Savannah? Do you still have commit access? (Although, posting your patches for me to apply would work too.) The steps do not have to retain full functionality -- later we can squash commits together to skip over the erring.

Benno Schulenberg <bens>
Project Administrator
Sat 21 Jul 2018 07:27:14 AM UTC, comment #4:

Oh, wow. I didn't realize that Pico had evolved a bit in the meantime. I have Pico 5.09 installed, and it too asks what to do when I hit ^J while some text is marked. (Strangely, when several paragraphs are marked, it justifies this region into a single paragraph.)

Anyway... I don't think we should be looking at Pico right now. Nor at FullJustify. All we need at first is that a single ^J becomes undoable in the normal way: through the undo stack. When that is possible, a series of ^Js will be undoable, and thus a full-justify too -- not in the way that one would like, but that is of later concern. Important is that one can make and undo a justification without losing the undo stack. What nano ought to do when the mark is on, etcetera, we can ignore for now. One step at a time.

Benno Schulenberg <bens>
Project Administrator
Fri 20 Jul 2018 01:57:48 AM UTC, comment #3:

Unfortunately, my schedule has been filling up lately too. I've looked into this, but I can't make any promises as to when I'll be able to do it. I can at least tell you what should be happening.

To do this, nano should be putting the paragraph(s) to be justified into their own buffer, altering the text within that buffer instead of within the main file buffer of fileage/current/filebot, and then grafting it back in. After that's the case, one would just need to use a temporary cutbuffer instead of a separate justify buffer, and then it should be undoable the same way the filtering operations are.

A normal justify and full justify could be treated almost identically, as the only real difference is that the former only justifies one paragraph, and the latter justifies multiple ones. One major problem is how to deal with the mark if the alterations are made to the justify buffer, but it may not be as bad as it seems.

If you look at Pico as included in Alpine 2.20, its behavior with regard to justify has changed somewhat from previous versions, in ways that make the job of imitating it somewhat easier. Specifically, if the mark is on, justify no longer shifts the mark relative to the text. Instead, it prompts: "Justify Region, Paragraph; or fix Quotelevel?"

Selecting "Region" justifies only the marked text, selecting "Paragraph" justifies the next paragraph it finds (what older Pico and current nano both do), but notably turns the mark off. Doing a full justify via ^W^U behaves the same (with one buggy behavior on Pico's part; if the mark is on, selecting "Region" will do nothing and stay at the prompt). Selecting "Quotelevel" prompts for a number, and then adds that number of ">" characters to the left of each line of the justified paragraph.

Given that displaying such a prompt while justifying doesn't fit nano at all, and nano's quotelevel handling is much more advanced than Pico's as-is, the easiest way to imitate current Pico when the mark is on is for nano to justify only within the marked text: the current paragraph within the mark for a normal justify, and all paragraphs within the mark for a full justify. This also means that mark adjustment will no longer be a problem when altering text in its own buffer while justifying.

This will, of course, be a notable change in behavior, but if properly documented, it should be easy to deal with.

David Lawrence Ramsey <dolorous>
Project Administrator
Wed 18 Jul 2018 06:11:02 PM UTC, comment #2:

If you are willing to work on this, that would be great. I have little time and energy to do much of anything these days. But I will test any patch that you make.

Benno Schulenberg <bens>
Project Administrator
Wed 18 Jul 2018 05:51:45 PM UTC, comment #1:

Given that filtering the buffer (or marked region) through an external command can be undone/redone now, the code for that could probably be adapted to undo/redo justifying as well.

After all, both filtering and justifying effectively replace one multi-line chunk of text with another multi-line chunk of text (and, unlike commenting and indenting multiple lines, the latter is not required to have the same number of lines as the former).

David Lawrence Ramsey <dolorous>
Project Administrator
Thu 14 Jan 2016 04:44:29 PM UTC, original submission:

Currently, when justifying a paragraph (^J), this can be undone with a special undo mechanism, only available right after the ^J. Also, to prevent wrong things from being undone/redone, the general undo stack is discarded.

It would be much better if the general undo system also covered the justifying routines.

Benno Schulenberg <bens>
Project Administrator

 

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

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dolorous (Posted a comment)
  • -email is unavailable- added by pepa65 (Voted in favor of this item)
  • -email is unavailable- added by bens (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-28 dolorous Attached File- => Added nano-justify.zip, #44648
    2018-07-27 dolorous Attached File- => Added 0001-justify-remove-redundant-checks-for-when-we-justifie.patch, #44633
        Attached File- => Added 0002-justify-refactor-to-use-justify_paragraph.patch, #44634
    2018-07-26 dolorous Attached File- => Added 0001-justify-refactor-to-use-justify_paragraph.patch, #44623
    2017-02-15 pepa65 Carbon-Copy- => Added pepa65

    Back to the top


    Powered by Savane 3.3