Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Update delete binding to use the isExplorerActive filter. #2162

Merged
merged 1 commit into from
May 3, 2018

Conversation

CrossR
Copy link
Member

@CrossR CrossR commented May 2, 2018

Updated the explorer delete binding to use the isExplorerActive filter, otherwise if you have a folder/file selected in it and hit delete (even whilst in insert mode!) you'll delete that folder.

I was getting random files being deleted whilst working and couldn't work out why. Luckily I have everything in git so it was no hassle, but could have been a lot worse. Since my keyboard has a function layer which sticks delete right on the home row, I use it a lot more than maybe other vim users who would need to move their hands to hit it.

What I'm less sure about is why I didn't hit this back when the delete was originally added, since it was added in Jan I think. Perhaps it was just because I was using the explorer less than I am now.

Copy link
Member

@akinsho akinsho left a comment

Choose a reason for hiding this comment

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

@CrossR it's great that you caught this, I didn't even realise there was a pre-existing binding for the delete functionality

I would never have known, I don't even have a delete key on my mac 😦

@bryphe
Copy link
Member

bryphe commented May 3, 2018

Updated the explorer delete binding to use the isExplorerActive filter, otherwise if you have a folder/file selected in it and hit delete (even whilst in insert mode!) you'll delete that folder.

Ouch, that's brutal! I'm glad you caught this, @CrossR ! 💯

@codecov
Copy link

codecov bot commented May 3, 2018

Codecov Report

Merging #2162 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2162      +/-   ##
==========================================
- Coverage   37.04%   37.04%   -0.01%     
==========================================
  Files         293      293              
  Lines       11980    11981       +1     
  Branches     1582     1582              
==========================================
  Hits         4438     4438              
- Misses       7290     7291       +1     
  Partials      252      252
Impacted Files Coverage Δ
browser/src/Input/KeyBindings.ts 2.19% <0%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2196aa0...163e6e5. Read the comment docs.

@CrossR CrossR merged commit eaf6695 into onivim:master May 3, 2018
@badosu
Copy link
Collaborator

badosu commented May 3, 2018

Is the last version released suffering from this bug? If so I think we should release a new version ASAP because this can have really nasty impacts.

@bryphe
Copy link
Member

bryphe commented May 3, 2018

Definitely, I agree - I just started a pre-release for 0.3.4 which has this and #2154

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

Successfully merging this pull request may close these issues.

4 participants