spacemacs icon indicating copy to clipboard operation
spacemacs copied to clipboard

* layers/+spacemacs/spacemacs-defaults/funcs.el: customer variable for spacemacs/delete-buffer-file

Open sunlin7 opened this issue 1 year ago • 8 comments

A customer variable to control the spacemacs/delete-current-buffer-file behavior to provide smoth change for users.

@smile13241324 @sunlin7 I have not pulled develop myself yet, but I'm afraid this might cause a quite annoying UX issue to users: Wouldn't this change cause users using previously learnt binding to suddenly delete without confirmation? If true, this would be a blatant UX no-no, IMO. See also: Principle of least astonishment

If proposed bindings are desirable, a more progressive rollout would prevent users to suffer it. I'd rather push initially just the binding change from SPC f D to SPC f d, leaving SPC f D for later, with just a "moved" message for some time to allow users to adjust. At a later time, we can rollout the SPC f D binding as intended here. The latter can even just be skipped completely if extra safety is desired for users, and left to them.

sunlin7 avatar May 16 '24 22:05 sunlin7

  • I'm unsure if "arbitrary" in the defcustom name conveys the meaning of "confirmation".
  • "User deletes the buffer and file without question." looks like to me a bit odd
  • I'd rather be more verbose/specific in the "to do it" message, e.g. "to delete it" or similar.

However, to prevent you from wasting any further effort, I'd like to get @smile13241324 take on the overall approach before moving forward. I'm not sure if adding yet another defcustom is the way to go or if there is another alternative solution which she'd like better.

pataquets avatar May 16 '24 22:05 pataquets

Updated with review comments.

sunlin7 avatar May 16 '24 23:05 sunlin7

I wonder what was bound to spc f d before, if nothing than spc f d and spc f D would be treated equally by Spacemacs this is ask for confirmation.

In this case I assume that nobody really used the capital version as its one press extra per use.

If this is so then the behaviour stays the same and no smoothening of the transition is necessary.

For the exceptional case that someone relied on this behaviour I would vote for adding a conf var to restore the old behaviour but not warn if it is used.

In addition we could add a warning on the splash screen under the rolling release message listing the breaking change and when it occurred.

Edit: I think this should warn most of the affected users, what do you think?

smile13241324 avatar May 17 '24 04:05 smile13241324

Hi @smile13241324 Thanks for your comments, I push new changes.

And I do searched the before, the SPC f d is not bind to any function. That's why shift the delete current buffer and file to SPC f d.

The SPC f D requests pressing SHIFT key can be regarded as another form of confirmating.

If I missed other key points, please let me know. Thanks.

sunlin7 avatar May 17 '24 06:05 sunlin7

SPC f d wasn't bound to anything before, so it's safe to use, I guess. I can confirm it, since I miss holding Shift from time to time and nothing happens :smile: I must admit that I like SPC f d best, as it's simpler and saves some finger-twisting, improving "RSI-ness". But changing SPC f D to no confirmation is dangerous. I disagree with @sunlin7's point about holding Shift as "a way to confirm", specially once you get used to it and becomes "muscle memory". You do it quite fast and without too much thinking, once you start the sequence. I use SPC f D quite often, so this is purely based on my own experience, but it's reasonable to assume that it can be a frequently used feature for some users. Disclaimer: getting used to this it's not such a big deal for me, and I expect to adjust fairly quick. But for users doing it more often can be dangerous. Thus my concerns about safety. If we can leave "D without confirmation" as an opt-in for now, maybe with a message stating that you can configure it to help users become aware of the new custom, we can finally set it as default in the future, after some prudential time has elapsed to give users more chances of discovering the upcoming default behaviour change.

pataquets avatar May 17 '24 08:05 pataquets

I strongly agree that we should not change the meaning of SPC f D without warning. Users will have this key sequence in muscle memory by now and expect it to prompt for confirmation. Changing its meaning should be gradual and opt-in, if we do it at all.

bcc32 avatar May 17 '24 17:05 bcc32

Hmmm sure we can do it this way, this would mean a second var to make spc f D non asking, with a standard value of nil.

@sunlin7 would you be willing to add this one too?

I think then the transition should be very smooth.

smile13241324 avatar May 17 '24 17:05 smile13241324

Sure! I pushed the the modified changes that intruduce a variable spacemacs-prompt-current-buffer-delete-bindings to do that. Please help review again. Thanks

sunlin7 avatar May 17 '24 18:05 sunlin7

@sunlin7, I think #16403 doesn't quite do what was agreed upon.

this would mean a second var to make spc f D non asking, with a standard value of nil.

The variable introduced actually has the opposite meaning but with a standard value of nil, so at the current tip of the develop branch, SPC f D by default does not prompt, which is a dangerous change in the behavior. Undoing the breaking change in #16423.

bcc32 avatar May 30 '24 05:05 bcc32

I readed your pr, and it's okay for me. Thanks

sunlin7 avatar May 30 '24 05:05 sunlin7