WerWolv / ImHex

🔍 A Hex Editor for Reverse Engineers, Programmers and people who value their retinas when working at 3 AM.
https://imhex.werwolv.net
GNU General Public License v2.0
43.96k stars 1.92k forks source link

impr: Added CSV, TSV and JSON as export options for Find results #1673

Closed SparkyTD closed 3 months ago

SparkyTD commented 4 months ago

Problem description

The default result export functionality of the Find tool is limited to only exporting data in a nonstandard text format. This PR adds support for exporting the results in CSV, TSV or JSON format. The PR also removes the old format.

Implementation description

I added the classes ExportFormatter, ExportFormatterCsv, ExportFormatterTsv and ExportFormatterJson, with similar implementations to the pattern data exporters.

I also moved the ViewFind::Occurrence class into hex/helpers/types.hh, so the exporters can access it.

Screenshots

image

Additional things

Another small change I made is moving the "{} entries found" line on the same line as the Search and Reset buttons. I think it looks cleaner this way, but if anyone disagrees, I can revert it.

SparkyTD commented 4 months ago

The CSV/TSV exporters have been tested by finding 5 character longs strings in /dev/urandom, and all the escaping logic seems to work as it's supposed to.

As for JSON, the nlohmann library already takes care of escaping strings.

SparkyTD commented 4 months ago

There is an issue that causes the format selector popup to stay visible behind the main window in some cases when the user clicks the parent window. The bug also exists in the original pattern export popup. I haven't been able to fix it yet.

image

WerWolv commented 4 months ago

Thanks a lot for the work! I'd suggest adding these exporter registration things to the Content Registry instead, that way other plugins can re-use the same infrastructure you already wrote to add their own formatters. The Occurrences struct as well because now it's being imported by basically every single file in ImHex which is probably not needed :D

The actual implementations of the formatters can stay in the builtin plugin like it is

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 0% with 98 lines in your changes are missing coverage. Please review.

Please upload report for BASE (master@dedd99f). Learn more about missing BASE report.

Files Patch % Lines
...content/export_formatters/export_formatter_csv.hpp 0.00% 30 Missing :warning:
plugins/builtin/source/content/views/view_find.cpp 0.00% 29 Missing :warning:
...ontent/export_formatters/export_formatter_json.hpp 0.00% 14 Missing :warning:
plugins/builtin/source/content/data_formatters.cpp 0.00% 12 Missing :warning:
lib/libimhex/source/api/content_registry.cpp 0.00% 5 Missing :warning:
...content/export_formatters/export_formatter_tsv.hpp 0.00% 4 Missing :warning:
lib/libimhex/include/hex/api/content_registry.hpp 0.00% 2 Missing :warning:
...ude/content/export_formatters/export_formatter.hpp 0.00% 2 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1673 +/- ## ======================================== Coverage ? 1.50% ======================================== Files ? 279 Lines ? 27088 Branches ? 14556 ======================================== Hits ? 409 Misses ? 26421 Partials ? 258 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

iTrooz commented 3 months ago

@WerWolv Do you think this is ready to be merged ?