lizmat / App-Rak

21st century grep / find / ack / ag / rg on steroids
Artistic License 2.0
152 stars 7 forks source link

--unique option is not being handled for CSV files #29

Closed Zer0-Tolerance closed 1 year ago

Zer0-Tolerance commented 1 year ago

Hi liz, another small bug with CSV and --unique ,I've applied my own patch to parsed CSV as array of hashes:

rak --csv-per-line '{ .<a> }' /tmp/test.csv -u
The --unique option is not being handled and will be ignored.
If you believe this to be incorrect, please report an issue with
https://github.com/lizmat/App-Rak/issues/new .

../../../../tmp/test.csv
1:1
2:4
3:1

while it's working for JSON:

rak --json-per-line '{ .<ip_str> }' /tmp/a.json -u
101.43.51.104
lizmat commented 1 year ago

could you post the CSV file you use?

Zer0-Tolerance commented 1 year ago

sure:

cat /tmp/test.csv
a,b,c,d
1,2,3,4
4,3,2,1
1,2,3,4
Zer0-Tolerance commented 1 year ago

to me it's just about options compatibility rather than about CSV content, it seems --unique is only compatible for JSON in the current code ?

lizmat commented 1 year ago

--unique should work with any source. E.g. a pattern like '*.words.Slip'

Zer0-Tolerance commented 1 year ago

ok then the behavior is not consistent between JSON and CSV hence the error message saying it 's not handled. I've looked at the code but I don't find the relevant code to produce unique items.

lizmat commented 1 year ago

that lives in the rak plumbing. It should just need to pass the --unique flag onto the rak sub

Zer0-Tolerance commented 1 year ago

at least can you reproduce the bug ?

lizmat commented 1 year ago

Probably, haven't looked at it yet. Wanted to push a release that will by default only look at actual text files, rather than by a set of known extensions. The latter confused a lot of people, and in hindsight, I don't blame them :-)

lizmat commented 1 year ago

It will probably be tomorrow: already did the weekly today, and two releases and other associated stuff. Going to have dinner in a mo, and then probably zonk out for the rest of the day :-)

But please keep those issues coming :-)

Zer0-Tolerance commented 1 year ago

hehe sure thing, regarding the by default looking at only text file why not but you'll have to use Filetype::Magic to guess if it's text. It's good to be able to search in binary file as well for utf16 strings (we're off topic a bit).

lizmat commented 1 year ago

It's actually using path-utils's path-is-text function

lizmat commented 1 year ago

Finally getting to it. Note that:

rak --csv-per-line '{ .<a> }'

will never work, as $_ is an array, not a hash inside the pattern

Zer0-Tolerance commented 1 year ago

Finally getting to it. Note that:

rak --csv-per-line '{ .<a> }'

will never work, as $_ is an array, not a hash inside the pattern

it's working for me because I've applied the patch in issue #27 to get each line as a hash.

lizmat commented 1 year ago

https://github.com/lizmat/App-Rak/commit/071be9523e makes --unique and other result options work for --csv-per-line