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
Limit line lengths #245
Limit line lengths #245
Conversation
5633b59
to
aa87517
Compare
@forgottenswitch Thanks for the PR! I really appreciate you taking the time to try to fix #129! :-) Unfortunately, I think there are significant problems with the patch as-is:
Before continuing, I think we should focus entirely on my first objection, (1), and avoid getting distracted with (2) or (3) until we actually decide that this is what we want. |
It seems very complex and error prone to do this for multiple matches on one very long line. Before I even consider supporting something like that, I would absolutely demand a well designed abstraction around it. I'd suggest we iterate on it in the issue tracker, and not move to the PR phase until we have a design that we're both happy with. I'd also suggest we wait to do this until I get more of #162 done, which I hope to include pulling the printer out of ripgrep proper and into its own crate. |
I would strongly urge you not to spend more time on this PR until we hammer out a lot more details and a proper specification. This feature is simply too complicated to do otherwise. |
What this implements instead of even left- and rightmost context, |
Possible fix for #129
Python to generate a test: