LongSoft / UEFITool

UEFI firmware image viewer and editor
BSD 2-Clause "Simplified" License
4.47k stars 632 forks source link

Search needs to be improved, both in UI and functional parts #320

Closed NikolajSchlej closed 1 year ago

NikolajSchlej commented 2 years ago

Inspired by the recent discussions, I think it's time to accept that the current search sucks and needs improvement in both function and UI/UX.

Related issues/PRs:

I'd like to use this issue a main discussion point on what exactly are the pains, how we all use search and what exactly it's lacking the most.

I'd also like to suggest retiring UEFIFind and moving its function to UEFIExtract, so we can reduce the number of tools from one GUI and one CLI, with eventual replacement of UEFIExtract, UEFIReplace and UEFIPatch by a single uefitool-cli once the build is ready, but it will likely require another issue about it later.

mikebeaton commented 2 years ago

Ré specifically hex pasting, apologies again for not creating an issue first.

Then, can I just mention that the current suggested implementation is still compatible with - format, e.g. 1234abcd-1234-1234-2234-aaaaaaaaaaab. It also is compatible with "\x11\x22" as long as the string is consistently in that format.

I don't think there are any endianness issues (perhaps surprisingly). If I paste a GUID (perhaps with {} decorations) into the GUID field, the search finds it. If I paste the same GUID into the hex pattern field, then (with the suggested implementation) it again pastes successfully, but the search does not find it - as the bytes are in the wrong order. In actual use (if anyone wants to try it), this turns out to be entirely unsurprising and expected!

Although it does have at least a whiff of the 'auto-magic', of course, I genuinely believe that is not that magic, really. As implemented, pasting into hex fields allows only hex digits, and ignores all else. If the number is hex, it just pastes it. If the number starts with 0x (or 0X) it cleans that. (Equally if it ends with 'h'.) If the number happens to have {{}} decorations, they are stripped. In short, I genuinely find that the feature behaviour is not surprising, in use - even if implemented directly/automatically in paste-to-hex.

It is indeed designed to support the workflow Vit mentions. If there is any canonical source of UEFI GUIDs, I suppose it is indeed the EDK-II source code. So a feature designed for ease of use with that is still not a priori ad hoc, I believe.

NikolajSchlej commented 2 years ago

Alright, let's merge it then, you guys convinced me.

NikolajSchlej commented 2 years ago

Still, the original issue of search sucking doesn't get resolved by that PR in full, and I'd like to still discuss some ways we can improve it further, or even if we need to improve it further given that there are already a plenty of software tools aimed at effective searching in files, and it's arguably better to not try to reinvent them, but to make UEFITool parsing tree accessible to them.

@orangecms or @yeggor would likely be huge proponents of teaching the parsing engine to output JSON/JSON5 in addition the already existing "Qt-based UI", "a crapload of directories", and "a single directory with a crapload of files" we can output today. This will make the engine more accessible to scripts and pipelines.

NikolajSchlej commented 2 years ago

On the other hand, there are already a plethora of good CLI-first or CLI-only tools to parse and modify UEFI images, but there are very few nativeUI ones, with UEFITool remaining the only OSS tool like that I know of (non-OSS are PhoenixTool, UEFI BIOS Editor and OEM tools). There's also a webUI one developed by @orangecms at https://fiedka.app