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

Fixes parsing of wdiff output #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wesalvaro
Copy link

Fixes #6

@junghans
Copy link
Owner

Thanks for the report and the fix, I will test this on OSX, whose sed is slightly different, and then merge it.

@junghans
Copy link
Owner

Hmm, OSX it says:

": RE error: repetition-operator operand invalid

replace .*? to something like .* works, so it seems BSD sed doesn't implement .*? even with -E.

@junghans
Copy link
Owner

junghans commented May 6, 2024

It might be better to replace the whole color_filter() with wdiff -w "$RED" -x "$OFF" -y "$GREEN" -z "$OFF".

@lemzwerg
Copy link
Contributor

lemzwerg commented May 9, 2024

Yes, I think so, too. AFAICS, only wdiff knows where the diffs start and end, and parsing the output of wdiff will fail, even with non-greedy regular expressions – you can always construct input that clashes with -] and siblings.

@lemzwerg
Copy link
Contributor

lemzwerg commented May 9, 2024

As a corollary it is probably necessary to remove the --filter option because it cannot be implemented reliably.

@junghans
Copy link
Owner

junghans commented May 9, 2024

Yes, I think so, too. AFAICS, only wdiff knows where the diffs start and end, and parsing the output of wdiff will fail, even with non-greedy regular expressions – you can always construct input that clashes with -] and siblings.

Or, we inject out own markers and replace those, but as you said one can always construct a clash.

As a corollary it is probably necessary to remove the --filter option because it cannot be implemented reliably.

Yeah not sure if anybody has ever used that.

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.

Line ending with { is flipped to }
3 participants