GNU nano - Bugs: bug #46900, justifying should be covered by...
You are not allowed to post comments on this tracker with your current authentication level.
bug #46900: justifying should be covered by the general undo system
Submitter: | Benno Schulenberg <bens> | ||
Submitted: | Thu 14 Jan 2016 04:44:29 PM UTC | ||
Votes: | 3 | ||
Severity: | 4 - Important | Status: | Fixed |
Assigned to: | dolorous | Open/Closed: | Closed |
Jump to the original submission
Mon 25 Mar 2019 01:18:13 PM UTC, comment #75: |
Benno Schulenberg <bens> |
Tue 20 Nov 2018 06:30:32 PM UTC, comment #74: Applied to master, and pushed. The main commit is d54840c1.
|
Benno Schulenberg <bens> |
Mon 19 Nov 2018 06:14:49 PM UTC, comment #73: The patch set looks good to me. Thank you. |
David Lawrence Ramsey <dolorous> |
Mon 19 Nov 2018 10:13:53 AM UTC, comment #72: Attached is the patch set that just makes justifications undoable and excludes the addition of justifying marked regions. Please verify whether you can live with the commit messages. Mostly I've just removed some repetitious sentences and rephrased things.
|
Benno Schulenberg <bens> |
Sun 18 Nov 2018 07:51:04 PM UTC, comment #71: I've come as far as your patch 0011. Patch 0012 to 0014 are not a necessary part of making justification undoable. They add new functionality and should be a separate patch series, to be applied later.
|
Benno Schulenberg <bens> |
Thu 15 Nov 2018 11:36:09 PM UTC, comment #70: No. As far as I can tell, the code should be working as is. If I've missed anything, testing by other people in ways I haven't thought of should shake out any problems.
|
David Lawrence Ramsey <dolorous> |
Thu 15 Nov 2018 06:33:06 PM UTC, comment #69: Any new tweaks of your patch set? Or can I apply the series in 3e.zip as is? |
Benno Schulenberg <bens> |
Thu 18 Oct 2018 09:46:15 PM UTC, comment #68: If proposed patch 0002 in bug #54861 is applied, the fill variable will be renamed to wrap_at, which will break patch 0002 of version 3e. The attached new version of patch 0002 will apply, however; just replace the corresponding patch in version 3e with it. |
David Lawrence Ramsey <dolorous> |
Tue 02 Oct 2018 06:38:30 PM UTC, comment #67: Attached new version 3e. Relative to 3d, it's resynced with git eb9111a, and it removes one erroneous change.
|
David Lawrence Ramsey <dolorous> |
Mon 01 Oct 2018 06:36:46 PM UTC, comment #66: I appreciate the help; thanks. And I hadn't actually noticed that you could vote on bugs until now. |
David Lawrence Ramsey <dolorous> |
Mon 01 Oct 2018 06:11:06 PM UTC, comment #65: Thanks for the update.
|
Benno Schulenberg <bens> |
Sun 30 Sep 2018 07:09:15 PM UTC, comment #64: Thank you.
|
David Lawrence Ramsey <dolorous> |
Sun 30 Sep 2018 06:52:26 PM UTC, comment #63: Okay, I've applied the first four patches -- the ones that don't change or break any behavior. Now comes the harder work of looking into the behavior changes. |
Benno Schulenberg <bens> |
Sat 29 Sep 2018 04:02:49 AM UTC, comment #62: Attached new version 3c. Relative to version 3b, it resyncs against git 0b63de3. |
David Lawrence Ramsey <dolorous> |
Fri 28 Sep 2018 02:55:41 PM UTC, comment #61: Attached new version 3b. Relative to version 3a, it resyncs against git bed9971, and adds a new patch 0020 (which removes references to the inability to undo justifications in the manual and info pages). |
David Lawrence Ramsey <dolorous> |
Tue 25 Sep 2018 08:52:53 PM UTC, comment #60: Attached new version 3a. Relative to version 3, it resyncs against git efa323e (so it works with your fix for bug #54728), and removes some unneeded variable usage in patch 0019 (using the buffer operations directly instead of the cutting code means that the mark isn't unset, so it doesn't need to be saved). |
David Lawrence Ramsey <dolorous> |
Tue 25 Sep 2018 04:18:46 PM UTC, comment #59: For the "fumbling code" I have created a new "bug" report: bug #54728. Please verify whether you agree with my reasoning there. |
Benno Schulenberg <bens> |
Tue 25 Sep 2018 02:51:21 PM UTC, comment #58: Attached new version 3 (since there are no more letters after 2z). It's still against git c545438. Relative to 2z, it updates patch 0019: your second patch to allow undo_update(PASTE) is melded for testing purposes, and the cursor positions before and after the user presses ^J are now properly restored on unjustify.
|
David Lawrence Ramsey <dolorous> |
Mon 24 Sep 2018 07:16:50 PM UTC, comment #57: Your second patch, added on top of mine, does seem to fix things, after some quick testing on my part. Thank you. And my typing isn't that good when I'm tired, either :)
|
David Lawrence Ramsey <dolorous> |
Mon 24 Sep 2018 06:14:08 PM UTC, comment #56: I meant: it does two undo adds. And: that it should not update an undo item. (Typing becomes poor when I'm tired.) |
Benno Schulenberg <bens> |
Mon 24 Sep 2018 06:10:37 PM UTC, comment #55: Hi David,
|
Benno Schulenberg <bens> |
Mon 24 Sep 2018 05:07:35 PM UTC, comment #54: Argh. Attached updated version 2z. It's still against git c545438. Relative to 2y, the hookup to the undo system in patch 0019 is simpler, and it also builds in tiny mode with --enable-justify again.
|
David Lawrence Ramsey <dolorous> |
Mon 24 Sep 2018 04:46:11 PM UTC, comment #53: Okay. Thanks again for the info.
|
David Lawrence Ramsey <dolorous> |
Sun 23 Sep 2018 06:07:31 AM UTC, comment #52: (The HARDMARK/SOFTMARK setting is only relevant when deciding whether a cursor movement (without Shift) should cancel the mark. It has no importance anywhere else.)
|
Benno Schulenberg <bens> |
Sun 23 Sep 2018 01:02:37 AM UTC, comment #51: Thanks for the info. That at least narrows it down. I'm not in front of my development machine right now, so I can't look at the code as a whole right now, but I think part of the problem is that the faked marked cut of the unjustified text doesn't set openfile->kind_of_mark to anything, so it's left as SOFTMARK by default (instead of HARDMARK), which may affect things. I'll look into it as soon as I can. |
David Lawrence Ramsey <dolorous> |
Sat 22 Sep 2018 09:18:08 AM UTC, comment #50: I looked at the behavior of your patch set two weeks ago, but couldn't "make chocolate from it" -- meaning I didn't understand what was happening when I pressed Undo.
|
Benno Schulenberg <bens> |
Thu 20 Sep 2018 08:36:49 PM UTC, comment #49: Attached new version. No changes from before except for a resync against git 790f985.
|
David Lawrence Ramsey <dolorous> |
Thu 13 Sep 2018 05:54:10 PM UTC, comment #48: Forget the part in comment #47 how the cursor isn't put back where it was before justifying. That's handled properly by the COUPLE_BEGIN/COUPLE_END parts, which were commented out; sorry for the confusion.
|
David Lawrence Ramsey <dolorous> |
Thu 13 Sep 2018 04:35:35 PM UTC, comment #47: I'm still looking into the last patch of the set, to try and fix the actual hookup to the undo system. I've locally commented out the bits of nano that add COUPLE_BEGIN and COUPLE_END, so the justify/unjustify should be two undo items: a cut followed by a paste. However, it actually ends up being 5(!) items. With the attached file (I would use a file included with nano, but I need something simple), try the following:
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 04:10:25 PM UTC, comment #46: Attached another version: 2x. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 08:06:44 AM UTC, comment #45: Attached one more version: 2w. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 07:16:48 AM UTC, comment #44: Attached one last version: 2v. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 06:20:22 AM UTC, comment #43: Argh. Attached version 2u. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 04:51:46 AM UTC, comment #42: And finally, attached version 2t. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 03:10:38 AM UTC, comment #41: Argh. Attached version 2s. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 02:56:35 AM UTC, comment #40: Attached version 2r. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Sep 2018 12:12:15 AM UTC, comment #39: Attached version 2q. It's still against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Mon 10 Sep 2018 09:58:11 PM UTC, comment #38: Attached version 2p. It's been resynced against git 5b12295.
|
David Lawrence Ramsey <dolorous> |
Sun 09 Sep 2018 03:25:20 PM UTC, comment #37: After some more thought, hypothetical justify-marked-text-like-newer-Pico should work like Pico's and justify the entire selection as though it's one paragraph. It could be useful in cases of odd formatting that keep ordinary justify from working, and it's the only way to justify everything in that fashion. As for what nano should do for a full-justify when the mark is on... I figure it should do the same, since, as I've said before, Pico's behavior of doing nothing is worse than redundancy with ordinary justify. (Also, my earlier proposal for it would involve justifying only part of a selection if the selection didn't start with a paragraph, in order to match all the other forms of justify. Checking for that case just to avoid marking the file as modified, as all the other forms do, would lead to a lot more code complexity.)
|
David Lawrence Ramsey <dolorous> |
Sat 08 Sep 2018 04:06:42 PM UTC, comment #36: For the record, if you're wondering why the last patch set still has the old unjustify code in it, it's because I'm not sure what to do with it. Otherwise, I would have removed it before trying to add the new unjustify code. Specifically, I'm concerned about how much of the old behavior we should keep.
|
David Lawrence Ramsey <dolorous> |
Fri 07 Sep 2018 05:17:23 PM UTC, comment #35: Attached version 2o, also against git 3270aac.
|
David Lawrence Ramsey <dolorous> |
Fri 07 Sep 2018 12:52:01 AM UTC, comment #34: Attached version 2n, resynced against git 3270aac.
|
David Lawrence Ramsey <dolorous> |
Sat 01 Sep 2018 12:56:43 AM UTC, comment #33: Attached version 2m, also against git 13615d0.
|
David Lawrence Ramsey <dolorous> |
Fri 31 Aug 2018 09:45:23 PM UTC, comment #32: Attached version 2l, also against git 13615d0.
|
David Lawrence Ramsey <dolorous> |
Fri 31 Aug 2018 09:07:52 PM UTC, comment #31: ...and forgot to mention one last change in 2k: when full-justifying, current_x is properly set to the end of the last line of the file, as is already done for justify. |
David Lawrence Ramsey <dolorous> |
Fri 31 Aug 2018 09:05:11 PM UTC, comment #30: Attached version 2k, which is resynced against 13615d0, and cleans up a few bits from previous versions.
|
David Lawrence Ramsey <dolorous> |
Fri 31 Aug 2018 05:49:45 PM UTC, comment #29: And, finally, attached version 2j, which is also against git 921f8ae. Relative to 2i, it makes find_paragraph() work on its own buffer. |
David Lawrence Ramsey <dolorous> |
Fri 31 Aug 2018 04:59:57 PM UTC, comment #28: And now attached version 2i, which is also against git git 921f8ae. Relative to 2h, it makes justify_paragraph() a bit simpler; the fix in 2h shouldn't use two pointers when one will do just as easily. |
David Lawrence Ramsey <dolorous> |
Fri 31 Aug 2018 04:15:18 PM UTC, comment #27: Looked at it in detail (and sorry I missed a few details of your bug report before). The problem was that par_len was the wrong variable to use to indicate the paragraph length, since it continually shifted. Good catch.
|
David Lawrence Ramsey <dolorous> |
Wed 29 Aug 2018 09:00:56 PM UTC, comment #26: Without looking at it in detail, I think it's probably some other effect of the temporarily removed bits of justify. The routine to move current to the end of the paragraph that the patch adds is really a temporary hack to make things not completely broken (and, as you can see, it's not perfect).
|
David Lawrence Ramsey <dolorous> |
Wed 29 Aug 2018 07:19:04 PM UTC, comment #25: Yes, the first paragraph is still properly justified (that's why I used --fill=90, to be able to see the change), but because the cursor goes to line 3, a second ^J just justifies the first paragraph again, and thus after the first ^J any further consecutive ^J has zero effect. The cursor stays "stuck" on the third line. |
Benno Schulenberg <bens> |
Wed 29 Aug 2018 05:29:48 PM UTC, comment #24: If you're using an 80x24 screen when you do that, the third paragraph of NEWS ends after the bottom of the screen. Since patch 0005 temporarily removes handling of edittop, this could be just a display glitch, caused by edittop's being in the wrong place after justifying. I've seen it sometimes while testing; going through the code with gdb shows that the text is properly justified.
|
David Lawrence Ramsey <dolorous> |
Wed 29 Aug 2018 04:32:52 PM UTC, comment #23: Patch 0005 says that current is still moved to the end of the paragraph, but... this appears not to be the case when I try to justify the NEWS file: src/nano --ignore --fill=92 +1 NEWS, and then type: ^J ^J ^J... The cursor stays stuck on the third line of the paragraph. |
Benno Schulenberg <bens> |
Tue 28 Aug 2018 04:14:48 PM UTC, comment #22: Version 2g is attached; it's against the same git version as 2f, 93c8e31.
|
David Lawrence Ramsey <dolorous> |
Tue 28 Aug 2018 01:13:31 AM UTC, comment #21: And, finally, version 2f; attached.
|
David Lawrence Ramsey <dolorous> |
Mon 27 Aug 2018 09:23:13 PM UTC, comment #20: And now version 2e; attached.
|
David Lawrence Ramsey <dolorous> |
Mon 27 Aug 2018 06:26:59 PM UTC, comment #19: Argh; make that version 2d. In this (attached) version, handling the case where the last line of the paragraph is the last line of the buffer now no longer duplicates code unnecessarily; the variable for that case is now named lastline_inpar, since filebot_inpar no longer makes sense now that it's using its own buffer; and unnecessary tweaks have been removed. |
David Lawrence Ramsey <dolorous> |
Mon 27 Aug 2018 05:50:26 PM UTC, comment #18: And now, I've updated to version 2c. Relative to 2b, it fixes a minor indentation error, and, more importantly, makes justify_paragraph() work on its own buffer while properly moving current to the end of the paragraph afterwards (although totsize handling and mark handling are still gone). Attached. |
David Lawrence Ramsey <dolorous> |
Mon 27 Aug 2018 03:49:25 AM UTC, comment #17: Added one more update: I'm calling it version 2b. It's against the same version of git as 2a, 80bea3e (although with the fix to bug #54573 applied first, to keep things sane).
|
David Lawrence Ramsey <dolorous> |
Sun 26 Aug 2018 11:42:37 PM UTC, comment #16: Argh. The added patch in the last update was supposed to adjust only one comment, but added an unnecessary code change. Not to mention, there was a typo in the comment.
|
David Lawrence Ramsey <dolorous> |
Sun 26 Aug 2018 03:55:56 PM UTC, comment #15: Made one last update to the refactor; attached. It's against git 80bea3e, and adds one last adjustment. Specifically, one comment is unclear about what last_par_line is set to when full-justifying if we run out of paragraphs (i.e. the last line of the file is not part of a paragraph); it's cleared up with the added patch.
|
David Lawrence Ramsey <dolorous> |
Sat 25 Aug 2018 11:14:34 PM UTC, comment #14: The refactor was getting unnecessarily complicated, plus it duplicated and/or reshuffled a few things unnecessarily or in ways that caused breakage.
|
David Lawrence Ramsey <dolorous> |
Thu 23 Aug 2018 07:17:26 PM UTC, comment #13: Worked on the justify refactor some more.
|
David Lawrence Ramsey <dolorous> |
Wed 22 Aug 2018 06:43:52 PM UTC, comment #12: I've started over on refactoring justify, in order to take a cleaner approach than last time. The justify code as it is now is similar to the line-indent code before it was hooked up to undo: two functions that should be separate (justify and full-justify) are coupled together in a way that saves lines of code, but also makes things much harder to work with. So I've started with that, which means that there is currently some duplication of code, but that should be fixed eventually in the process of making things undoable.
|
David Lawrence Ramsey <dolorous> |
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.)
|
David Lawrence Ramsey <dolorous> |
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> |
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.
|
David Lawrence Ramsey <dolorous> |
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.
|
David Lawrence Ramsey <dolorous> |
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.
|
David Lawrence Ramsey <dolorous> |
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.
|
David Lawrence Ramsey <dolorous> |
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.
|
Benno Schulenberg <bens> |
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.)
|
Benno Schulenberg <bens> |
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.
|
David Lawrence Ramsey <dolorous> |
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> |
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.
|
David Lawrence Ramsey <dolorous> |
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.
|
Benno Schulenberg <bens> |
Depends on the following items: None found
Items that depend on this one: None found
There are 3 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 25 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2019-03-25 | bens | Open/Closed | Open | Closed | |
2018-11-20 | bens | Status | In Progress | Fixed | |
2018-11-19 | bens | Attached File | - | Added undoable-justification.zip, #45486 | |
2018-11-18 | bens | Attached File | - | Added justification-reworked.zip, #45483 | |
2018-11-15 | bens | Severity | 3 - Normal | 4 - Important | |
2018-10-18 | dolorous | Attached File | - | Added 0002-justify-make-justify_paragraph-work-on-its-own-buffe.patch, #45241 | |
2018-10-02 | dolorous | Attached File | - | Added nano-justify-3e.zip, #45125 | |
2018-10-01 | bens | Attached File | - | Added 0003-moving-make-the-paragraph-movement-functions-work-on.patch, #45123 | |
2018-09-30 | dolorous | Attached File | - | Added nano-justify-3d.zip, #45121 | |
2018-09-29 | dolorous | Attached File | - | Added nano-justify-3c.zip, #45116 | |
2018-09-28 | dolorous | Attached File | - | Added nano-justify-3b.zip, #45113 | |
2018-09-25 | dolorous | Attached File | - | Added nano-justify-3a.zip, #45099 | |
2018-09-25 | dolorous | Attached File | - | Added nano-justify-3.zip, #45093 | |
2018-09-24 | bens | Attached File | - | Added report-the-adds-and-updates.patch, #45087 | |
Attached File | - | Added allow-a-paste-item-to-be-updated.patch, #45088 | |||
Status | None | In Progress | |||
Assigned to | None | dolorous | |||
2018-09-24 | dolorous | Attached File | - | Added nano-justify-2z.zip, #45085 | |
2018-09-24 | dolorous | Attached File | - | Added nano-justify-2y.zip, #45084 | |
2018-09-20 | dolorous | Attached File | - | Added nano-justify-2x.zip, #45067 | |
2018-09-13 | dolorous | Attached File | - | Added justify-small-test.txt, #45000 | |
2018-09-11 | dolorous | Attached File | - | Added nano-justify-2x.zip, #44981 | |
2018-09-11 | dolorous | Attached File | - | Added nano-justify-2w.zip, #44979 | |
2018-09-11 | dolorous | Attached File | - | Added nano-justify-2v.zip, #44978 | |
2018-09-11 | dolorous | Attached File | - | Added nano-justify-2u.zip, #44977 |
Powered by Savane 3.13-758e.
Corresponding source code
Released in 4.0.