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

[WIP] Implement -o, --only-matching. #125

Conversation

utkarshkukreti
Copy link

Fixes #34.

@BurntSushi The tests pass, but let me know if anything is wrong or can be improved!

@BurntSushi
Copy link
Owner

BurntSushi commented Sep 28, 2016

@utkarshkukreti Thanks! This looks good as a first pass. One of the reasons why I hadn't just done this myself is because I wasn't sure about its interaction with other flags. But we can figure that out here! Some questions:

  • What if the user wants line counts? What about column numbers? What about file paths?
  • What if --vimgrep is passed? (I think having one flag override the other is fine btw, we just need to say it.)
  • What about colors?
  • How does it interact with the context options? (GNU grep's output here is... strange, but I can see how it makes sense. Namely, it doesn't print any context lines, but does still print context separators.)

Pretty much all of these interactions need to be tested too. Doing some of these things may not be very nice in the current printer code.

@utkarshkukreti
Copy link
Author

@BurntSushi thanks for the initial review! I'll try to address everything as soon as I can. As for (3), both ag and grep color all the text with -o, so I think we should do the same here if colors are enabled.

@utkarshkukreti utkarshkukreti changed the title Implement -o, --only-matching. [WIP] Implement -o, --only-matching. Sep 28, 2016
@BurntSushi
Copy link
Owner

BurntSushi commented Sep 28, 2016

@utkarshkukreti That's a reasonable choice.

In general, we should probably put a strong preference towards answering my questions by looking at what other tools do. I've tried to give grep behavior the higher priority over any other tool. (With that said, there can be good reasons for doing something different than grep, we should just try to justify it.)

@utkarshkukreti
Copy link
Author

Unfortunately, I didn't get time to work on this and probably won't for a few weeks. I'm closing this but if anyone wants to use these changes as a starting point for their own attempt, please feel free to!

@BurntSushi
Copy link
Owner

@utkarshkukreti No problem! Thanks for the heads up! I understand this was probably trickier than it appeared, in large part because the printing code isn't in the best shape. I expect to try to add this when I refactor the search/print code.

BurntSushi added a commit that referenced this pull request Nov 6, 2018
This commit fixes a bug where AsciiDoc would drop any line containing a
'{foo}' because it interpreted it as an undefined attribute reference:

> Simple attribute references take the form {<name>}. If the attribute name
> is defined its text value is substituted otherwise the line containing the
> reference is dropped from the output.

See: https://www.methods.co.nz/asciidoc/chunked/ch30.html

We fix this by simply replacing all occurrences of '{' and '}' with
their escaped forms: '&#123;' and '&#125;'.

Fixes #1101
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.

None yet

2 participants