search

benma / visual-regexp.el

A regexp/replace command for Emacs with interactive visual feedback
392 stars 28 forks source link

Inconsistent history value with `query-replace` #37

Closed cute-jumper closed 8 years ago

cute-jumper commented 8 years ago

vr/query-replace-defaults-variable's default value is query-replace-defaults, which will be set to a list of cons pairs after performing some replacements, for example, (("A" . "b") ("B" . "b")). However, query-replace only store a cons pair in query-replace-defaults, i.e., ("A" . "b"). This inconsistency makes it impossible to use them together. After using one of them, we can't use another one.

Silex commented 8 years ago

I have the same problem, it was probably introduced in https://github.com/benma/visual-regexp.el/commit/80668e93223ad54d0c4c8facc1bac1cdf2155f4c

Silex commented 8 years ago

Ok, the format of this variable changed in emacs25 and the new code assumes only emacs 25.

@benma: given you are the most familiar with the change, you probably know better where to add the right if checking for version < 25 :)

conornash commented 8 years ago

I'm seeing this error too. Where would a change need to be made to submit a patch?

Silex commented 8 years ago

Probably around https://github.com/benma/visual-regexp.el/blob/master/visual-regexp.el#L689 https://github.com/benma/visual-regexp.el/blob/master/visual-regexp.el#L701 and https://github.com/benma/visual-regexp.el/blob/master/visual-regexp.el#L718

Some wrapping with (if (version< emacs-version "25") foo bar) or similar

benma commented 8 years ago

Fixed in https://github.com/benma/visual-regexp.el/commit/2cf4dc5a2dff0736eb2e2da95997d7274bbb5766. Please let me know if that helps.

For Emacs 24, the defaults of visual regexp and the builtin replace are not now independent (and interoperable), but at least using vr does not break the builtin functions anymore. In Emacs 25, the defaults variable is shared between vr and the builtin replace and fully compatible.

Silex commented 8 years ago

Nice :+1: I confirm the problem is gone on my end.

conornash commented 8 years ago

Looks like it's time for me to upgrade to Emacs 25 :)

benma commented 8 years ago

Looks like it's time for me to upgrade to Emacs 25 :)

I for one did not have any issues with upgrading, so give it a try :page_facing_up:

@cute-jumper can you confirm this problem is solved for you as well?

conornash commented 8 years ago

Ah yes, I tried using the master branch instead of the exact commit you linked. Fixed for me in Emacs 24. Thank you!

benma commented 8 years ago

What do you mean? The commit I linked is master.

cute-jumper commented 8 years ago

@benma Perfect. Thank you.