Really protect against unwanted indentation after undo #119
Conversation
This protects against other similar "indent on very next command" problems similar to the one described in Malabarba#119. * aggressive-indent.el (aggressive-indent--proccess-changed-list-and-indent): Clear aggressive-indent--changed-list. (aggressive-indent--keep-track-of-changes): Don't check undo-in-progress
This protects against other similar "indent on very next command" problems similar to the one described in Malabarba#119. * aggressive-indent.el (aggressive-indent--proccess-changed-list-and-indent): Clear aggressive-indent--changed-list. (aggressive-indent--keep-track-of-changes): Don't check undo-in-progress
|
Thanks for the effort João. As it stands, I think the solution is too aggressive. IIUC, this code clears out all previously stored changes, even if they are older than the undo. |
|
Too aggressive for aggressive-indent? It makes it less aggressive! :) Jokes aside, can you describe a case where my approach wouldn't work or is it just a feeling? |
|
Right, I think I see the problem. Perhaps I can add a unique marker to the change list in the post-command-book and not forget past that marker. Is that ok? |
|
@Malabarba I changed my approach completely. It seems simpler and less disruptive. There is still the off-chance, I think that clearing the changes, which happens in the EDIT: this does make sense after all |
|
Nevermind, it seems to be buggy too... EDIT: It's not |
|
Nevermind the nevermind, it is correct after all. I'll just take care of a minor detail in a further commit. |
|
@Malabarba that's it, uff. You can review this now :-) |
|
@Malabarba ping? I'm sorry about the hesitation, but the current mode seems to be working currently in my testing, and is still a fairly surgical change. |
|
This patch has made aggressive-indent much more predictable for me. I would recommend merging it. |
aggressive-indent.el
Outdated
|
|
||
| Clears `aggressive-indent--changed-list' iff the current | ||
| command (the one that's now finished) lives in | ||
| `aggressive-indent-protected-current-commands'." |
There was a problem hiding this comment.
Typo. It's actually the other variable that the code is using.
|
Sorry for taking so long, and thanks a lot for your effort João. Just fix the typo and we're ready to merge. |
|
@joaotavora Fix that typo and rebase and this can be merged in. |
Sorry. I'm not using |
|
But I can rebase the PR, that's easy to do. |
|
So I just rebased it. If you can find out what the "other variable" is, i.e.e how that comment should be, just tell me and I'll do it. |
|
Replace The |
|
OK
…On Fri, Dec 13, 2019 at 1:06 AM Troy Hinckley ***@***.***> wrote:
Replace aggressive-indent-protected-current-commands with
aggressive-indent-protected-commands.
The current part is the typo.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#119?email_source=notifications&email_token=AAC6PQZL674HB2SKMCZCJITQYLGZ7A5CNFSM4GFFL35KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGYO2CY#issuecomment-565243147>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC6PQ6K7G6AA7PD5J5BQJLQYLGZ7ANCNFSM4GFFL35A>
.
--
João Távora
|
When I am undoing changes to a region I really want the changes to be undone verbatim, that is, I want the buffer's contents to be like they were before the change that I want to undo occured. This is the very definition of undo. It is probably the rationale for giving undo special treatment in aggressive-indent--internal-dont-indent-if in the first place. It means that the undo command won't immediately cause reindentation. But it doesn't fix the whole problem, because changes performed by undo itself are still recorded into aggressive-indent--changed-list and the very next command, be it a move or an edit somewhere else, will still cause those regions to be indented. Among other things, it's impossible in practice to use `undo' to undo an aggressive indent of a region. The proposed fix checks adds a function to the post-command hook to check for aggressive-indent-protected-commands and clear the change list if the current command is in that list. This is safe if those changes can only have been performed by the command whose auto-indentation effects we want to prevent. * aggressive-indent.el (aggressive-indent--keep-track-of-changes): Check undo-in-progress. * aggressive-indent.el (aggressive-indent--internal-dont-indent-if): Remove check of aggressive-indent-protected-commands and undo-in-progress. (aggressive-indent--post-command): New helper. (aggressive-indent-mode): Put aggressive-indent--post-command in post-command-hook and remove it when exiting minor mode.
* aggressive-indent.el (aggressive-indent--pre-command-change-head): New variable. (aggressive-indent--pre-command): New function. (aggressive-indent-mode): Add aggressive-indent--pre-command to pre-command-hook and remove it when exiting minor mode.
|
@Malabarba this is ready to merge |
When I am undoing changes to a region I really want the changes to be
undone verbatim, i.e. I want the buffer's contents to be as they were before
the change I undid (this is the very definition of undo).
This is probably the rationale for have undo-in-progress in
aggressive-indent--internal-dont-indent-ifin the first place.But it doesn't fix the whole problem, because changes performed by
undo are still recorded into
aggressive-indent--changed-listand thevery next command will indent those regions. This means it's
impossible in practice to use
undoto undo an aggressive indent of aregion.
The fix proposed here checks
undo-in-progressbefore registering achange. It's possible that other elements (but maybe not all) in
aggressive-indent--internal-dont-indent-ifmerit this treatment, too.Check undo-in-progress.