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 target-groups option #117

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

Conversation

TheophileMot
Copy link

The idea here is to let the user specify which groups should be matched for the purposes of colouring. If unspecified, then use the existing behaviour (i.e., iterate through all groups).

This addresses several problems:

  • Sometimes groups are only in the regex for the purpose of pattern matching, not colouring;
  • Nested groups will repeat part of the string in the output;
  • Regex using + or * after a group will only capture the last instance of the group, so to capture the whole thing, the group must be nested inside another.

The code works, but I don't really know Python at all so I ended up just making target-groups a global variable. This is presumably not the right way to do it, but it should be easy to fix for someone who knows better.

Example:

# BEFORE:

$ echo 'a1 b2 c3 123-456-7890 9z 8q 7w hello' | ./colout.py '([a-z]\d )+(\d+-)*\d+ (\d[a-z] )*(\w+)' blue,yellow,none,red
Highlights only 'c3' instead of 'a1 b2 c3', etc.

$ echo 'a1 b2 c3 123-456-7890 9z 8q 7w hello' | ./colout.py '(([a-z]\d )+)((\d+-)*\d+) ((\d[a-z] )*)(\w+)' blue,yellow,none,red
This is an attempt to address the first problem by wrapping in another layer of groups; however, it produces garbled output, and the colours are wrong because they cycle through unwanted groups.

# PROPOSED SOLUTION:

$ echo 'a1 b2 c3 123-456-7890 9z 8q 7w hello' | ./colout.py '(([a-z]\d )+)((\d+-)*\d+) ((\d[a-z] )*)(\w+)' blue,yellow,red -tg 1 3 7
We get the expected output by specifying the groups we want. Also note that the colour 'none' is no longer necessary, since we can just omit that group from the list.

@nojhan
Copy link
Owner

nojhan commented Oct 27, 2023

Would you prefer a Github review and edit yourself the changes I would suggest, or should I add a commit changing what I want directly?

@nojhan nojhan self-assigned this Oct 27, 2023
@TheophileMot
Copy link
Author

Thanks for having a look at this, and I hope the idea is useful. Please go ahead and directly make whatever changes you deem necessary; I will be interested in seeing your edits afterward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants