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.

Sign up for GitHub

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

Jump to bottom

fix: make 'rm -rf foo/' be a bit harder #14749

Closed
wants to merge 1 commit into from
Closed

fix: make 'rm -rf foo/' be a bit harder #14749

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions runtime/autoload/netrw.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11454,24 +11454,29 @@ fun! s:NetrwLocalRmFile(path,fname,all)

else
" attempt to remove directory
let rmfile= substitute(rmfile,'[\/]$','','e')
Copy link
Member

Choose a reason for hiding this comment

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

I would leave the directory indicator. That is actually useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was there before this PR and #14742
i guess it was remained there for judging if it was a dir or not, i just kept it.

Copy link
Member

Choose a reason for hiding this comment

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

No, this was not touched by #14742

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, so i didnot plan to touch it either.
// please notice that if/else

if !all
echohl Statement
call inputsave()
let ok= input("Confirm *recursive* deletion of directory<".rmfile."> ","[{y(es)},n(o),a(ll),q(uit)] ")
" make it be a bit harder to make mistake by inputting upper case
let ok= input("Confirm *recursive* deletion of directory<".rmfile."> ","[{Y(ES)},n(o),A(LL),q(uit)] ")
Copy link
Member

Choose a reason for hiding this comment

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

Now you are unexpectedly changing the key-bindings. This will break peoples habits. I don't think this is appreciated by long-time users. (besides, it actually looks funny, that some keys need to be upper-cased and others not).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, i am not against just back out the #14742, then kept the appreciated long-time users' habits, or you had better idea not to so easy or high possibility to make mistake to rm all?

Copy link
Member

Choose a reason for hiding this comment

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

What mistake, there is still a confirmation dialog

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. lower case is easy to mis-type, like a 'hit-enter' happened.
  2. it was designed to: if 'all=1' then no 'confirm', else req a confirm, and confirm to only delete('rm', 'd') not 'rf'.

call inputrestore()
let ok= substitute(ok,'\[{y(es)},n(o),a(ll),q(uit)]\s*','','e')
let ok= substitute(ok,'\[{Y(ES)},n(o),A(LL),q(uit)]\s*','','e')
if ok == ""
let ok="no"
endif
if ok =~# 'a\%[ll]'
if ok =~# 'a\%[ll]'->toupper()
let all= 1
endif
endif
let rmfile= substitute(rmfile,'[\/]$','','e')

if all || ok =~# 'y\%[es]' || ok == ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also please note: i removed this ok == "" though not sure when it would happen (or mostly it would not happen), but it scared me if i just pressed 'enter' somehow.

if delete(rmfile,"rf")
call netrw#ErrorMsg(s:ERROR,"unable to delete directory <".rmfile.">!",103)
if all || ok =~# 'y\%[es]'->toupper()
if delete(rmfile,"rf")
call netrw#ErrorMsg(s:ERROR,"failed to *recursive* delete directory <".rmfile.">!",103)
endif
endif
let ok = ok->tolower()
else
if delete(rmfile,"d")
call netrw#ErrorMsg(s:ERROR,"unable to delete directory <".rmfile."> -- is it empty?",103)
endif
endif
endif
Expand Down
Loading