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

Some minor fixed #26

Merged
merged 7 commits into from
Jul 28, 2019
Merged

Some minor fixed #26

merged 7 commits into from
Jul 28, 2019

Conversation

Ergus
Copy link
Contributor

@Ergus Ergus commented Jun 22, 2019

  • Fixed issue when mark-even-if-inactive is nil.
  • Forced kbd macro evaluation to fix C-g that was not working.

@coveralls
Copy link

coveralls commented Jun 22, 2019

Coverage Status

Coverage decreased (-11.07%) to 86.585% when pulling 9c3aaea on Ergus:master into f5e6cf4 on paldepind:master.

@Ergus
Copy link
Contributor Author

Ergus commented Jun 22, 2019

Hi, I am trying this and the changes are really minor issues, So I don't understand why some test fail. Maybe the C-g change in the test needs a similar change than in the code? Because C-g was not working before.

in Travis it says for the failing test:

  Scenario: Cancel with C-g
    When I insert "first second third"
    And I place the cursor before "second"
    And I press "C-w"
    And I press "C-g"   <- Failed here
    And I press "a"
    Then I should see "first asecond third"

But manually this scenario works for me. Please could you give a look and accept when fixed?

Changed mark-active check for mark-active-p.

Also the mark is properly deactivated when exiting the
composable-object-mode.
kbd macros where not executed, so, the combinations C-g was unset.
This change enables to track the number of times a command was repeater.
@Ergus Ergus marked this pull request as ready for review June 22, 2019 18:13
@Ergus
Copy link
Contributor Author

Ergus commented Jun 25, 2019

@paldepind
Hi I just added a simple fancy functionality to change the modeline background color and I tested everything and it seems to work in my system. So the failing test I am pretty sure is a test error, because without my changes the command is bind to C - g instead of C-g. Could you please give a look and merge my changes or suggest corrections?
Best regards, and thanks in advance.

@paldepind
Copy link
Owner

He @Ergus. Thank you for the PR. I have exams at the moment but I'll take a look as soon as I get the time.

This fixes the need of repeating commands without undoing the previous
one.
It is more efficient for copies and fixes issue paldepind#22.
@Ergus
Copy link
Contributor Author

Ergus commented Jul 13, 2019

Hi I added some more changes to fix the issue #22 (because I faced it).

It seems to be working, in spite of Travis shows errors in the tests, but when I try the actions manually they produce the expected output. Could you please check this when you have some time?

@Ergus
Copy link
Contributor Author

Ergus commented Jul 26, 2019

Hi:

I just added a new commit to solve #20 with which-key integration. I actually wrote to the which-key developer to add an API to do this See

I also modified the Readme to inform about the changes.

Could you please give a look and accept of fix the changes to be integrated?

@paldepind
Copy link
Owner

Looks super great @Ergus. Thanks a lot. The fixes are nice and the which-key integration is really useful.

Thank again 👍

@paldepind paldepind merged commit b2139cd into paldepind:master Jul 28, 2019
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.

3 participants