GNU nano - Bugs: bug #54042, running an external spell check...
You are not allowed to post comments on this tracker with your current authentication level.
bug #54042: running an external spell check should be undoable
Submitter: | Benno Schulenberg <bens> | ||
Submitted: | Sun 03 Jun 2018 09:32:56 AM UTC | ||
Severity: | 3 - Normal | Status: | Fixed |
Assigned to: | dolorous | Open/Closed: | Closed |
Jump to the original submission
Mon 10 Sep 2018 05:45:18 PM UTC, comment #23: |
Benno Schulenberg <bens> |
Wed 08 Aug 2018 06:09:51 PM UTC, comment #22: No problem. |
David Lawrence Ramsey <dolorous> |
Wed 08 Aug 2018 06:00:22 PM UTC, comment #21: Okay, I think it is now good enough for wider testing. The whole lot has been pushed to master, commits b5f15cb5 to 94b4f072.
|
Benno Schulenberg <bens> |
Wed 08 Aug 2018 05:53:44 PM UTC, comment #20: Argh. Forgot about the statting; good catch.
|
David Lawrence Ramsey <dolorous> |
Wed 08 Aug 2018 05:43:17 PM UTC, comment #19: Well, the code for statting the temp file is already present: around line 2912 (after applying the patch set). But for the current setup, the check comes too late: the file has already been marked as modified by the new "cutting and pasting". What should be done is: move the stat further up, and not do any replacing and other fiddling at all when the temp file was not changed.
|
Benno Schulenberg <bens> |
Wed 08 Aug 2018 12:14:53 AM UTC, comment #18: Of those two, not having any undo item would be much trickier to accomplish, since it would involve manually altering the undo stack (as in other hypothetical cases of undone unchanged text that you've brought up), and I'm still trying to figure out some parts of the undo code. And even then, you run into the same problem that you get when e.g. trying to undo cutting the magicline: even if no text has changed, the cursor position where you tried to do something to the text needs to be saved, which requires some kind of undo item, at least the way things are currently designed.
|
David Lawrence Ramsey <dolorous> |
Tue 07 Aug 2018 07:21:47 PM UTC, comment #17: The one thing that doesn't work right is when a spell check finishes immediately because the external spell checker does not find any misspellings. In that case there should not be any undo item, and the file should not be marked as modified. |
Benno Schulenberg <bens> |
Tue 07 Aug 2018 06:58:30 PM UTC, comment #16: Good catch on COUPLE_END; thanks. After some quick testing, your reordered patch set works as far as I can tell, and the squashing/commit message tweaks in it are fine with me. |
David Lawrence Ramsey <dolorous> |
Tue 07 Aug 2018 06:46:38 PM UTC, comment #15: Here is a reordered set, with the first two patches squashed together and tweaked commit messages. Please verify. |
Benno Schulenberg <bens> |
Tue 07 Aug 2018 06:22:36 PM UTC, comment #14: Thanks. (The COUPLE_END for execute_command() needs the same setting of the "filtering" string, of course.)
|
Benno Schulenberg <bens> |
Tue 07 Aug 2018 03:14:54 PM UTC, comment #13: No problem. Back now, a little sooner than I thought.
|
David Lawrence Ramsey <dolorous> |
Mon 06 Aug 2018 05:18:15 PM UTC, comment #12: No worries. There is no hurry. |
Benno Schulenberg <bens> |
Mon 06 Aug 2018 12:48:04 AM UTC, comment #11: Unfortunately, I've been busier than I thought, so it'll be a few more days until I can get back to my development machine. Thanks for the undo/redo messages code, though; I'll look at both attempts as soon as I can. |
David Lawrence Ramsey <dolorous> |
Sat 04 Aug 2018 08:06:20 AM UTC, comment #10: And here is the way that I suggested at first. What do you like best? |
Benno Schulenberg <bens> |
Sat 04 Aug 2018 07:57:04 AM UTC, comment #9: Here is what I fumbled together to get proper undo/redo messages for an external spellcheck.
|
Benno Schulenberg <bens> |
Thu 02 Aug 2018 01:27:00 AM UTC, comment #8: Good to know. I'm not certain how to set the undo/redo messages, but I'll look into it. It'll be a few days before I can get to it, though. |
David Lawrence Ramsey <dolorous> |
Wed 01 Aug 2018 06:20:40 PM UTC, comment #7: Thanks for the patch set. It works, as far as I can tell after a quick test. Just one thing: it should not say that "Filtering" has been undone, but instead that "Spelling corrections" have been undone. (Maybe it is possible to store the undo/redo message in the COUPLE_BEGIN and COUPLE_END items?) |
Benno Schulenberg <bens> |
Tue 31 Jul 2018 09:10:06 PM UTC, comment #6: Okay. I fixed the build when using --enable-tiny --enable-speller, and resynced against git ac102a7; the new version is attached. Or is it major enough that I should send it to the mailing list?
|
David Lawrence Ramsey <dolorous> |
Tue 31 Jul 2018 05:54:37 PM UTC, comment #5: Putting back the cursor is not essential for making an external spell check (or a justification) undoable. Of course, not restoring the cursor position is a bug, but something not being undoable is a bigger bug -- and discarding all the previous undo items is a major bug. So, don't worry about the cursor position for now. |
Benno Schulenberg <bens> |
Tue 31 Jul 2018 02:25:07 AM UTC, comment #4: Thoughts on comment #2: The cut-to-eof used when adding an undo for alt spell-check has to change the cursor position from where the user pressed Ctrl-T to the beginning of the file, so that the entire file is properly cut, and the initial cursor position ends up lost. If I understand the code correctly, saving the initial cursor position somewhere in the undo (possibly as part of COUPLE_BEGIN?) would fix this problem.
|
David Lawrence Ramsey <dolorous> |
Mon 30 Jul 2018 10:23:51 PM UTC, comment #3: Another caveat: it probably needs NANO_TINY #ifdefs around the added undo code for when a user tries --enable-tiny --enable-speller, but I don't have any more time to check that at the moment. |
David Lawrence Ramsey <dolorous> |
Mon 30 Jul 2018 10:17:36 PM UTC, comment #2: One caveat: with these applied, when nano undoes an alt spell-check, it doesn't put the cursor back exactly where it was when the user hit Ctrl-T. This should be fixed, but I'm not sure how yet. |
David Lawrence Ramsey <dolorous> |
Mon 30 Jul 2018 10:08:24 PM UTC, comment #1: Attached an attempt at this, made against git c231b77. It's modeled after undo for external command filtering, in that it uses the cutbuffer to discard text before replacing it. The marked spell check does a marked cut, and the unmarked spell check does a cut-to-eof from the top of the file.
|
David Lawrence Ramsey <dolorous> |
Sun 03 Jun 2018 09:32:56 AM UTC, original submission:
Currently, after an external spell check, the undo stack is discarded and nor the spelling corrections nor any earlier edits are undoable any more.
|
Benno Schulenberg <bens> |
Depends on the following items: None found
Items that depend on this one: None found
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 11 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2018-09-10 | bens | Open/Closed | Open | Closed | |
2018-08-08 | bens | Status | In Progress | Fixed | |
2018-08-08 | bens | Attached File | - | Added statbefore.patch, #44739 | |
2018-08-07 | bens | Attached File | - | Added newset.zip, #44737 | |
2018-08-07 | dolorous | Attached File | - | Added 0007-undo-distinguish-between-general-filtering-and-spell.patch, #44735 | |
2018-08-04 | bens | Attached File | - | Added 0007-undo-distinguish-between-general-filtering-and-spe.patch, #44703 | |
2018-08-04 | bens | Attached File | - | Added 0007-undo-differentiate-between-general-filtering-and-spe.patch, #44702 | |
2018-08-01 | bens | Status | None | In Progress | |
Assigned to | None | dolorous | |||
2018-07-31 | dolorous | Attached File | - | Added nano-speller.zip, #44668 | |
2018-07-30 | dolorous | Attached File | - | Added nano-speller.zip, #44663 |
Powered by Savane 3.13-02a9.
Corresponding source code
Released in 3.0. Thanks again.