Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double undo #100

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix double undo #100

wants to merge 4 commits into from

Conversation

Kypert
Copy link

@Kypert Kypert commented Apr 18, 2020

Including fix of #98, although it is tough to follow the history of the undojoin, it seems to be the source of the double undo, at least for neovim (v0.5.0-427-g1f56f9a4b) and vim (8.0 1-124).

Tried to create a unit test for the same, however, it seems like the undo-list is behaving in a surprising way (probably me missing something).

When the user adds invalid arguments to the -style switch of
clang-format, the error message may yield different responses depending
on the type of failure.

Expand error message given to the user to not only be verbose on 'key'
errors.

An example:

    let g:clang_format#style_options = { "Standard" : "go" }

Will result in:

    Error detected while processing function clang_format#replace[6]..<SNR>77_error_message:
    line    1:
    clang-format has failed to format.
    YAML:1:63: error: unknown enumerated scalar
    {BasedOnStyle: LLVM, IndentWidth: 4, UseTab: false, Standard: 'go'}
                                                                  ^~~~
    Error parsing -style: Invalid argument
Comment on lines +465 to +469
" When running :ClangFormat from a normal vim instance, setline() does add an entry in the undolist
" For some reason it does not do so when running in this test framework
" Also, seems like it is hard to even add another item to the change list too, always rendering:
" # number changes when saved
" # 9 1 0 seconds ago
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be happy if someone could enlighten me regarding this comment.

Avoid add unnecessary items to the change-list if the code is already
formatted according to clang-format.

Add a cheerful echo to praise the user (positive nudging?). However,
some users might not be pleased by that, so introduce a way to disable
the joyful echo.
Kind of hard to follow the history of this, but currently (NVIM
v0.5.0-427-g1f56f9a4b) it seems like this undojoin makes some unexpected
behaviour for the user. Unable to tell what-else is unlocked by
removing it, so let's try without it for a while.
@e4lam
Copy link

e4lam commented May 1, 2020

What is the actual fix for the double undo here? There's several different changes mixed into this PR.

@Kypert
Copy link
Author

Kypert commented May 1, 2020

I suppose the core of the PR would be c6cc7d2 (the one marked with the issue). Still, it feels more like a revert of some older fix (maybe not needed any longer, or only valid for vim?).

Sneakily added the others as a bonus, although perhaps bad practice, I felt like they are separated enough in each commit.

@e4lam
Copy link

e4lam commented May 2, 2020

Ok thanks! I'll try applying it to my copy of this. I found this PR because I noticed that this bug as well.

@e4lam
Copy link

e4lam commented May 2, 2020

Oh so, this is basically just removing this one line:
silent! undojoin

@Kypert
Copy link
Author

Kypert commented May 2, 2020

Yes, seems to work for me, but not sure about all consequences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants