Open kazarmy opened 7 years ago
A proper parser should be used. Welcome to contribute syntax of commands at https://hackmd.io/GYBghgzAjBBGCcBaArAdgqxAWKATKi8AxmAQKYAcqZIqYxAbA8kA
I agree in principle. Regarding the link:
QuotedStatement = Statement - Redirection - TmpSeek + ';'
Shouldn't a QuotedStatement
have, um, quotes?
Statement = Head {Arg} Redirection? TmpSeek? Grep?
I would put the Redirection?
at the end, since it makes more sense to me to constrain the output first before redirecting it.
before this thing goes too far, i will not accept bison/yacc in r2
On 19 Sep 2017, at 13:21, Khairul Azhar Kasmiran notifications@github.com wrote:
I agree in principle. Regarding the link:
QuotedStatement = Statement - Redirection - TmpSeek + ';' Shouldn't a QuotedStatement have, um, quotes?
Statement = Head {Arg} Redirection? TmpSeek? Grep? I would put the Redirection? at the end, since it makes more sense to me to constrain the output first before redirecting it.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/radare/radare2/issues/8555#issuecomment-330508965, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-lg9QdpAHB2VRsmD37jDOBMSW5PZnks5sj6OrgaJpZM4PbD5j.
This issue has been automatically marked as stale because it has not had recent activity. Considering a lot has changed since its creation, we kindly ask you to check again if the issue you reported is still relevant in the current version of radare2. If it is, update this issue with a comment, otherwise it will be automatically closed if no further activity occurs. Thank you for your contributions.
Still relevant but on the back burner for the time being.
Since we now have cfg.newshell
I think this should be resolved properly by parsing the grep syntax directly with tree-sitter.
For all tests to pass, still need to fix both the old and new parsers though.
We can just use e cfg.newshell=true
in the test. I don't think it's worth our time to focus on 2 implementations. I think cfg.newshell
is the future.
this was fixed already for some cases, but not all of them. the problem of parsing this is not because of newshell or not newshell. the command output is filtered in RCons and the bug is there.
I added a PR with a broken test exemplifying the failing cases https://github.com/radareorg/radare2/pull/17086
The reason why I'm saying that it should be done with newshell
is because right now to make grep work you must flatten things in one single grep. So you never have "nested" greps, but you just have the illusion of it (please correct me if i'm wrong, i haven't checked the code but just going from memory).
So, pd 10~word1~word2
actually gets rewritten as pd 10~word1,word2
, so that it can be parsed as a single grep.
The problem is that as soon as you mess up a bit with the command, it doens't work anymore. E.g. pd 10~lea~:0..2~rcx
doesn't work anymore. With a proper parser instead this would be interpreted something like:
(grep_command
(grep_command
(grep_command
(arged_command ...) # pd 10
(grep_specifier ...) # lea
(grep_specifier ...) # :0..2
(grep_specifier ...))))) # rcx
And you would just need to implement the single filter operation on the output of the previous command (no matter if it comes from another grep_command, from a regular arged_command or something else).
Yes, i do agree, the current RConsGrep struct doesnt handle such nesting logic, it flats everything into lists of filterings that must be applied one after the other. and no
pd 10~word1~word2
is translated to pd 10~&word1,word2
(comma separated grep words are taken as OR, not AND)
pd 10~word1~word2
is translated topd 10~&word1,word2
(comma separated grep words are taken as OR, not AND)
Yes sure, my bad.
Currently
cmd~word~word2
works for apparently any amount of~
. Combinations of~
and~!
e.g.cmd~word~!word2
should also be supported.