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.

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

Add interactive highlights for ex global command pattern #1875

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

blahgeek
Copy link

No description provided.

@tomdl89
Copy link
Member

tomdl89 commented Mar 30, 2024

Hi @blahgeek thanks for the PR. How does your implementation compare to that in https://github.com/mamapanda/evil-traces? Could code for other highlighting be brought over? Could recommending evil-traces in the readme be preferable? I'm not sure if you're aware of its existence, and if not, if knowing about it would have changed your approach.

@tomdl89
Copy link
Member

tomdl89 commented Mar 30, 2024

Ah, and I guess more importantly, does your functionality and that of evil-traces conflict at all?

@blahgeek
Copy link
Author

Ah, I did not know the existence of evil-traces. Let me take a took and get back to you. Thanks!

@blahgeek
Copy link
Author

I briefly tried evil-traces and skimmed through its code, here's what I found:

Evil-traces does already cover the feature of this patch, along with the support for many other commands. Its implementation is similar to this patch, and it would override current implementation (so no conflict).

However I do find several edge cases that evil-traces is incorrect:

  • It does not handle patterns with \c or \C correctly
  • It does not handle vim style patterns (e.g. with \d) correctly
  • It does not support ex-global within visual range (:'<,'>g/)

Surely these issues can be fixed in evil-traces, but IMO it would be more likely to work correctly in the long run being implemented in evil package itself (with more integrated codes and tests).

Ultimately I think it depends on whether you think this feature (and possibly other highlights features) belong to evil core package. IMO, since evil already provides substitution highlight feature, it's nature to also provide ex-global-command highlight feature, but other highlight features maybe not so much.

@blahgeek blahgeek force-pushed the ex-global-highlights branch from f606ec9 to fc79578 Compare April 14, 2024 07:44
@blahgeek blahgeek force-pushed the ex-global-highlights branch from fc79578 to 898c985 Compare August 26, 2024 03:53
@blahgeek blahgeek force-pushed the ex-global-highlights branch from 898c985 to 4d4d698 Compare December 16, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants