From 9375a60cd9d183de2987d679253ffe74baf09fb4 Mon Sep 17 00:00:00 2001 From: BioTheWolff <47079795+BioTheWolff@users.noreply.github.com> Date: Mon, 16 Dec 2024 20:35:48 +0100 Subject: [PATCH] feat: Implement paste behaviour popup when pasting over one-byte regions (#2004) ### Problem description This PR implements the feature request described in #1995, that describes a problem with the `Paste` vs `Paste all` commands. Users could be thrown off by having `Ctrl+V` act as a simple "Paste over selection", whereas it's generally accepted as a "Paste all". ### Implementation description This PR introduces a new setting, called "Paste behaviour" (under the "Hex Editor" category). This setting has three values: - `Paste over selection`: the current implementation for ImHex. Pastes only over the selection region, in this case pasting only one byte; - `Paste everything`: allows ImHex's `Paste` to behave like a `Paste all` when selecting one-byte regions; - `Ask me next time`: prompts the user for a choice of behaviour (default value). *Note: as users generally use `Paste all` when selecting one-byte regions, calling `Paste` when selecting over two or more bytes is not affected by this change, and will still behave like the usual `Paste` command.* When selecting a one-byte region, and calling the Paste command, users that have not defined a preferred behaviour in the settings will be prompted to choose one, using a brand new popup. The popup also allows the user to cancel, which will not change the settings' value, and will cancel the paste action altogether. ### Screenshots The new popup: ![image](https://github.com/user-attachments/assets/2b0fd532-d4e7-4209-9dd7-8a79278692ea) The new setting: ![image](https://github.com/user-attachments/assets/2644c35e-7332-422e-8fae-ae8ad0507126) ### Additional things I'm not very good with long descriptions, so I'm open to any suggestions regarding the text that is included in the popup! I do think however that we should keep a hint indicating that `Paste all` is always an option, which could solve the issue altogether for very new users. --------- Signed-off-by: BioTheWolff <47079795+BioTheWolff@users.noreply.github.com> Co-authored-by: Nik --- .../include/content/views/view_hex_editor.hpp | 6 ++ plugins/builtin/romfs/lang/en_US.json | 6 ++ .../source/content/settings_entries.cpp | 6 ++ .../source/content/views/view_hex_editor.cpp | 75 ++++++++++++++++++- 4 files changed, 91 insertions(+), 2 deletions(-) diff --git a/plugins/builtin/include/content/views/view_hex_editor.hpp b/plugins/builtin/include/content/views/view_hex_editor.hpp index 70ee16ef9..2dd996390 100644 --- a/plugins/builtin/include/content/views/view_hex_editor.hpp +++ b/plugins/builtin/include/content/views/view_hex_editor.hpp @@ -80,6 +80,12 @@ namespace hex::plugin::builtin { void registerEvents(); void registerMenuItems(); + /** + * Method dedicated to handling paste behaviour when using the normal "Paste" option. + * Decides what to do based on user settings, or opens a popup to let them decide. + */ + void processPasteBehaviour(const Region &selection); + ui::HexEditor m_hexEditor; bool m_shouldOpenPopup = false; diff --git a/plugins/builtin/romfs/lang/en_US.json b/plugins/builtin/romfs/lang/en_US.json index f90e0782b..0cfa16420 100644 --- a/plugins/builtin/romfs/lang/en_US.json +++ b/plugins/builtin/romfs/lang/en_US.json @@ -490,6 +490,7 @@ "hex.builtin.setting.hex_editor.char_padding": "Extra character cell padding", "hex.builtin.setting.hex_editor.highlight_color": "Selection highlight color", "hex.builtin.setting.hex_editor.pattern_parent_highlighting": "Highlight pattern parents on hover", + "hex.builtin.setting.hex_editor.paste_behaviour": "Single-Byte Paste behaviour", "hex.builtin.setting.hex_editor.sync_scrolling": "Synchronize editor scroll position", "hex.builtin.setting.imhex": "ImHex", "hex.builtin.setting.imhex.recent_files": "Recent Files", @@ -819,6 +820,11 @@ "hex.builtin.view.hex_editor.menu.edit.open_in_new_provider": "Open selection view...", "hex.builtin.view.hex_editor.menu.edit.paste": "Paste", "hex.builtin.view.hex_editor.menu.edit.paste_as": "Paste...", + "hex.builtin.view.hex_editor.menu.edit.paste.popup.title": "Choose paste behaviour", + "hex.builtin.view.hex_editor.menu.edit.paste.popup.description": "When pasting values into the Hex Editor View, ImHex will only overwrite the bytes that are currently selected. If only a single byte is selected however, this can feel counter-intuitive. Would you like to paste the entire content of your clipboard if only one byte is selected or should still only the selected bytes be replaced?", + "hex.builtin.view.hex_editor.menu.edit.paste.popup.hint": "Note: If you want to ensure pasting everything at all times, the 'Paste all' command is also available in the Edit Menu!", + "hex.builtin.view.hex_editor.menu.edit.paste.popup.button.everything": "Paste everything", + "hex.builtin.view.hex_editor.menu.edit.paste.popup.button.selection": "Paste only over selection", "hex.builtin.view.hex_editor.menu.edit.paste_all": "Paste all", "hex.builtin.view.hex_editor.menu.edit.paste_all_string": "Paste all as string", "hex.builtin.view.hex_editor.menu.edit.remove": "Remove...", diff --git a/plugins/builtin/source/content/settings_entries.cpp b/plugins/builtin/source/content/settings_entries.cpp index 526f6ad3e..fd7567a53 100644 --- a/plugins/builtin/source/content/settings_entries.cpp +++ b/plugins/builtin/source/content/settings_entries.cpp @@ -835,6 +835,12 @@ namespace hex::plugin::builtin { ContentRegistry::Settings::add("hex.builtin.setting.hex_editor", "", "hex.builtin.setting.hex_editor.pattern_parent_highlighting", true); + std::vector pasteBehaviourNames = { "Ask me next time", "Paste everything", "Paste over selection" }; + std::vector pasteBehaviourValues = { "none", "everything", "selection" }; + ContentRegistry::Settings::add("hex.builtin.setting.hex_editor", "", "hex.builtin.setting.hex_editor.paste_behaviour", + pasteBehaviourNames, + pasteBehaviourValues, + "none"); } /* Fonts */ diff --git a/plugins/builtin/source/content/views/view_hex_editor.cpp b/plugins/builtin/source/content/views/view_hex_editor.cpp index f42c44b4f..e5587f61c 100644 --- a/plugins/builtin/source/content/views/view_hex_editor.cpp +++ b/plugins/builtin/source/content/views/view_hex_editor.cpp @@ -483,6 +483,47 @@ namespace hex::plugin::builtin { std::string m_input; }; + class PopupPasteBehaviour final : public ViewHexEditor::Popup { + public: + explicit PopupPasteBehaviour(const Region &selection, const auto &pasteCallback) : m_selection(), m_pasteCallback(pasteCallback) { + m_selection = Region { .address=selection.getStartAddress(), .size=selection.getSize() }; + } + + void draw(ViewHexEditor *editor) override { + const auto width = ImGui::GetWindowWidth(); + + ImGui::TextWrapped("%s", "hex.builtin.view.hex_editor.menu.edit.paste.popup.description"_lang.get()); + ImGui::TextUnformatted("hex.builtin.view.hex_editor.menu.edit.paste.popup.hint"_lang); + + ImGui::Separator(); + + if (ImGui::Button("hex.builtin.view.hex_editor.menu.edit.paste.popup.button.selection"_lang, ImVec2(width / 4, 0))) { + m_pasteCallback(m_selection, true); + editor->closePopup(); + } + + ImGui::SameLine(); + if (ImGui::Button("hex.builtin.view.hex_editor.menu.edit.paste.popup.button.everything"_lang, ImVec2(width / 4, 0))) { + m_pasteCallback(m_selection, false); + editor->closePopup(); + } + + ImGui::SameLine(ImGui::GetWindowWidth() - ImGui::GetCursorPosX() - (width / 6)); + if (ImGui::Button("hex.ui.common.cancel"_lang, ImVec2(width / 6, 0))) { + // Cancel the action, without updating settings nor pasting. + editor->closePopup(); + } + } + + [[nodiscard]] UnlocalizedString getTitle() const override { + return "hex.builtin.view.hex_editor.menu.edit.paste.popup.title"_lang; + } + + private: + Region m_selection; + std::function m_pasteCallback; + }; + /* Hex Editor */ ViewHexEditor::ViewHexEditor() : View::Window("hex.builtin.view.hex_editor.name", ICON_VS_FILE_BINARY) { @@ -730,6 +771,36 @@ namespace hex::plugin::builtin { provider->write(selection.getStartAddress(), buffer.data(), size); } + void ViewHexEditor::processPasteBehaviour(const Region &selection) { + if (selection.getSize() > 1) { + // Apply normal "paste over selection" behaviour when pasting over several bytes + pasteBytes(selection, true, false); + return; + } + + // Selection is over one byte, we have to check the settings to decide the course of action + + auto setting = ContentRegistry::Settings::read( + "hex.builtin.setting.hex_editor", + "hex.builtin.setting.hex_editor.paste_behaviour", + "none"); + + if (setting == "everything") + pasteBytes(selection, false, false); + else if (setting == "selection") + pasteBytes(selection, true, false); + else + this->openPopup(selection, + [](const Region &selection, const bool selectionCheck) { + ContentRegistry::Settings::write( + "hex.builtin.setting.hex_editor", + "hex.builtin.setting.hex_editor.paste_behaviour", + selectionCheck ? "selection" : "everything"); + pasteBytes(selection, selectionCheck, false); + }); + + } + static void copyString(const Region &selection) { auto provider = ImHexApi::Provider::get(); if (provider == nullptr) @@ -1190,8 +1261,8 @@ namespace hex::plugin::builtin { /* Paste */ ContentRegistry::Interface::addMenuItem({ "hex.builtin.menu.edit", "hex.builtin.view.hex_editor.menu.edit.paste" }, ICON_VS_OUTPUT, 1450, CurrentView + CTRLCMD + Keys::V, - [] { - pasteBytes(ImHexApi::HexEditor::getSelection().value_or( ImHexApi::HexEditor::ProviderRegion(Region { 0, 0 }, ImHexApi::Provider::get())), true, false); + [this] { + processPasteBehaviour(ImHexApi::HexEditor::getSelection().value_or( ImHexApi::HexEditor::ProviderRegion(Region { 0, 0 }, ImHexApi::Provider::get()))); }, ImHexApi::HexEditor::isSelectionValid, this);