search

Malabarba / aggressive-indent-mode

Emacs minor mode that keeps your code always indented. More reliable than electric-indent-mode.
850 stars 36 forks source link

Really protect against unwanted indentation after undo #119

Open joaotavora opened 5 years ago

joaotavora commented 5 years ago

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-if in the first place.

But it doesn't fix the whole problem, because changes performed by undo are still recorded into aggressive-indent--changed-list and the very next command will indent those regions. This means it's impossible in practice to use undo to undo an aggressive indent of a region.

The fix proposed here checks undo-in-progress before registering a change. It's possible that other elements (but maybe not all) in aggressive-indent--internal-dont-indent-if merit this treatment, too.

Malabarba commented 5 years ago

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.

joaotavora commented 5 years ago

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?

joaotavora commented 5 years ago

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?

joaotavora commented 5 years ago

@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 post-command-hook clears some change performed by a previous command. We could circumvent this by marking the most recent change in pre-command-hook and not clearing past that one.

EDIT: this does make sense after all

joaotavora commented 5 years ago

Nevermind, it seems to be buggy too... EDIT: It's not

joaotavora commented 5 years ago

Nevermind the nevermind, it is correct after all. I'll just take care of a minor detail in a further commit.

joaotavora commented 5 years ago

@Malabarba that's it, uff. You can review this now :-)

joaotavora commented 5 years ago

@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.

CeleritasCelery commented 5 years ago

This patch has made aggressive-indent much more predictable for me. I would recommend merging it.

Malabarba commented 5 years ago

Sorry for taking so long, and thanks a lot for your effort João. The code looks very surgical indeed. :-)

Just fix the typo and we're ready to merge.

CeleritasCelery commented 4 years ago

@joaotavora ping

Fix that typo and rebase and this can be merged in.

joaotavora commented 4 years ago

Fix that typo and rebase and this can be merged in.

Sorry. I'm not using a-i-m anymore and I don't have time to confirm Artur's comment. But you can make a pull request that includes my commit and another one on top of it that fixes the typo, and explains why.

joaotavora commented 4 years ago

But I can rebase the PR, that's easy to do.

joaotavora commented 4 years ago

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.

CeleritasCelery commented 4 years ago

Replace aggressive-indent-protected-current-commands with aggressive-indent-protected-commands.

The current part is the typo.

joaotavora commented 4 years ago

OK

On Fri, Dec 13, 2019 at 1:06 AM Troy Hinckley notifications@github.com 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 https://github.com/Malabarba/aggressive-indent-mode/pull/119?email_source=notifications&email_token=AAC6PQZL674HB2SKMCZCJITQYLGZ7A5CNFSM4GFFL35KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGYO2CY#issuecomment-565243147, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC6PQ6K7G6AA7PD5J5BQJLQYLGZ7ANCNFSM4GFFL35A .

-- João Távora

CeleritasCelery commented 4 years ago

@Malabarba this is ready to merge