benma / visual-regexp.el

A regexp/replace command for Emacs with interactive visual feedback
400 stars 29 forks source link

Allow C-u to switch the case sensitivity [proposal] #35

Closed parkouss closed 8 years ago

parkouss commented 8 years ago

Hello, I was replacing some things and had to be cautious with the case.

M-x toggle-case-fold-search helped me, but I had to type that again to remove the behaviour since it applies globally.

Maybe allowing C-u to switch the case would be interesting? I am thinking about something like that (this works for me, though that could be done directly in visual-regexp if that make sense to you):


(defun my-replace ()
  (interactive)
  (let ((case-fold-search (if current-prefix-arg (not case-fold-search)
                            case-fold-search)))
    (call-interactively 'vr/replace)))
nispio commented 8 years ago

It might make more sense to be able to toggle the behavior by adding a binding to vr/minibuffer-regexp-keymap because it does not require that you exit your search if you forget (or don't know beforehand) that you want modified case behavior.

benma commented 8 years ago

vr/replace tries to behave just like query-replace-regexp. I don't think the latter has some sort of prefix or keybinding to toggle case-fold-search, please correct me if I am wrong.

Because of that, I am reluctant to add this sort of feature. I think private snippets in your configuration like the one you posted are a good fit for the problem, and the reason why emacs is awesome: you can customize anything :smile:

What do you think?

Side note: I also have a personal customization of the prefix arg; in my case, I use it to toggle between the emacs and python engines in visual-regexp-steroids.

nispio commented 8 years ago

In general, I don't think that adding an additional configuration option works against the goal of being a drop-in replacement for another feature. The current implementation of vr/replace does not support the prefix argument the way that query-replace-regexp does, but I agree that it would be a mistake to use the prefix argument for something different than that command does.

As a reference, I can toggle case sensitivity in vr/isearch-forward because the isearch-mode-map provides a command isearch-toggle-case-fold-search bound to M-c.

Alternatively, it would help to have a hook that runs at the end of vr/replace so that you could restore the value of case-fold-search when it is done.

benma commented 8 years ago

I think adding the feature by extending case-fold-search in visual-regexp via vr/minibuffer-regexp-keymap, like @nispio suggests, is acceptable. Would you be willing to code it up? In visual-regexp-steroids' Python engine, the keybinding is C-c i, maybe we should add M-c as well?

@parkouss, @nispio, do you think a lot of people would find it useful? If it is only a tiny minority, I still think adding this keybinding in your private configuration is enough.

The current implementation of vr/replace does not support the prefix argument the way that query-replace-regexp does,

That is right, the reason being mostly that no one cares about it ;) I would accept PRs to fix that, though.

Hmm, vr/isearch-forward is a steroids thing to make Python regexp work. For non-emacs engines, I don't strive to be compatible with Emacs (sometimes it does not even make sense). In that vein, I am unsure about the state of case-fold-search there. I'd be surprised if it works.

nispio commented 8 years ago

@benma: I'm actually not that interested in the feature myself since I use the steroids' python engine anyway. But if I was going to implement it as a personal keybinding it would be nice to have a hook that runs at the end of vr/replace so that I could restore the value of case-fold-search when I was done.

nispio commented 8 years ago

Pull request #36 is more of a proof-of-concept than an actual request to merge. I'm not sure whether it is worth the added complexity if nobody is clamoring for it. It seems to work as expected though.

parkouss commented 8 years ago

@benma, I am not sure if a lot of people would find that useful, and I am totally fine to keep it in my personal configuration.

I agree that doing something different than the way query-replace-regex works would be a bad idea, and the mode map solution seems a lot more elegant. Though I don't know if it worth the cost of the extra complexity, as @nispio said (thanks btw for working on that anyway!).

So, I have no strong opinion here.

benma commented 8 years ago

@nispio

But if I was going to implement it as a personal keybinding it would be nice to have a hook that runs at the end of vr/replace so that I could restore the value of case-fold-search when I was done.

I don't think there is a need for a hook. You can defadvice vr/replace or wrap it in your own function to restore the value.

Thanks for the PR. Seeing that both of you are fine with not adding the feature, would it be okay if you close it again and we close this issue? (Also pinging @parkouss).

parkouss commented 8 years ago

Closing the issue is totally fine for me. :)

benma commented 8 years ago

OK. Thanks guys ;)

wyuenho commented 4 years ago

@benma @nispio I'd definitely find that useful. Can you reopen the PR?