Skip to content

Commit d8dd287

Browse files
paxcutpaxcut
authored
fix: Fixed ImHex crashing when using ctrl-backspace on empty file. (WerWolv#2433)
Editor was attempting to delete non-existent chars which is UB. Fixed by checking before deleting. Also fixed was a problem created by having to press enter to change the search string which advanced the selection to the first match. In the next step one would expect that pressing enter on the replace field would replace the selected item but was replacing the item found after he first. This was fixed by always replacing the current selection first. If the replacement is the same as the searched term then replacing won't advance the cursor, but if they are different then the current match will no longer exist so it would search fora new one.
1 parent d62b64c commit d8dd287

File tree

10 files changed

+122
-89
lines changed

10 files changed

+122
-89
lines changed

‎.github/workflows/build.yml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -473,15 +473,15 @@ jobs:
473473
set -x
474474
mkdir -p build
475475
cd build
476-
CC=$(brew --prefix llvm)/bin/clang \
477-
CXX=$(brew --prefix llvm)/bin/clang++ \
478-
OBJC=$(brew --prefix llvm)/bin/clang \
479-
OBJCXX=$(brew --prefix llvm)/bin/clang++ \
476+
CC=$(brew --prefix llvm@20)/bin/clang \
477+
CXX=$(brew --prefix llvm@20)/bin/clang++ \
478+
OBJC=$(brew --prefix llvm@20)/bin/clang \
479+
OBJCXX=$(brew --prefix llvm@20)/bin/clang++ \
480480
PKG_CONFIG_PATH="$(brew --prefix openssl)/lib/pkgconfig":"$(brew --prefix)/lib/pkgconfig" \
481481
cmake -G "Ninja" \
482482
-DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \
483483
-DIMHEX_GENERATE_PACKAGE=ON \
484-
-DIMHEX_SYSTEM_LIBRARY_PATH="$(brew --prefix llvm)/lib;$(brew --prefix llvm)/lib/unwind;$(brew --prefix llvm)/lib/c++;$(brew --prefix)/lib" \
484+
-DIMHEX_SYSTEM_LIBRARY_PATH="$(brew --prefix llvm@20)/lib;$(brew --prefix llvm@20)/lib/unwind;$(brew --prefix llvm@20)/lib/c++;$(brew --prefix)/lib" \
485485
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
486486
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
487487
-DCMAKE_OBJC_COMPILER_LAUNCHER=ccache \

‎cmake/build_helpers.cmake‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ macro(setupCompilerFlags target)
744744
endif()
745745

746746
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND APPLE)
747-
execute_process(COMMAND brew --prefix llvm OUTPUT_VARIABLE LLVM_PREFIX OUTPUT_STRIP_TRAILING_WHITESPACE)
747+
execute_process(COMMAND brew --prefix llvm@20 OUTPUT_VARIABLE LLVM_PREFIX OUTPUT_STRIP_TRAILING_WHITESPACE)
748748
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -L${LLVM_PREFIX}/lib/c++")
749749
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -L${LLVM_PREFIX}/lib/c++")
750750
addCCXXFlag("-Wno-unknown-warning-option" ${target})

‎dist/macOS/Brewfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ brew "freetype2"
66
brew "libmagic"
77
brew "pkg-config"
88
brew "curl"
9-
brew "llvm"
9+
brew "llvm@20"
1010
brew "glfw"
1111
brew "ninja"
1212
brew "zlib"

‎plugins/builtin/source/content/views/view_pattern_editor.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ namespace hex::plugin::builtin {
904904
static std::string replaceWord;
905905
static bool downArrowReplace = false;
906906
static bool upArrowReplace = false;
907-
if (ImGui::InputTextWithHint("##replaceInputTextWidget", hint.c_str(), replaceWord, replaceFlags) || downArrowReplace || upArrowReplace) {
907+
if (ImGui::InputTextWithHint("###replaceInputTextWidget", hint.c_str(), replaceWord, replaceFlags) || downArrowReplace || upArrowReplace) {
908908
findReplaceHandler->setReplaceWord(replaceWord);
909909
historyInsert(m_replaceHistory, m_replaceHistorySize, m_replaceHistoryIndex, replaceWord);
910910

@@ -972,7 +972,8 @@ namespace hex::plugin::builtin {
972972

973973
if ((ImGui::IsKeyPressed(ImGuiKey_F3, false)) || downArrowFind || upArrowFind || enterPressedFind) {
974974
historyInsert(m_findHistory, m_findHistorySize, m_findHistoryIndex, findWord);
975-
position = findReplaceHandler->findMatch(textEditor, !shift && !upArrowFind);
975+
i32 index = !shift && !upArrowFind ? 1 : -1;
976+
position = findReplaceHandler->findMatch(textEditor, index);
976977
count = findReplaceHandler->getMatches().size();
977978
updateCount = true;
978979
downArrowFind = false;
@@ -2021,9 +2022,9 @@ namespace hex::plugin::builtin {
20212022
ContentRegistry::UserInterface::addMenuItem({ "hex.builtin.menu.file", "hex.builtin.view.pattern_editor.menu.find_next" }, 1520, AllowWhileTyping + Keys::F3, [this] {
20222023
if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {
20232024
ui::TextEditor::FindReplaceHandler *findReplaceHandler = editor->getFindReplaceHandler();
2024-
findReplaceHandler->findMatch(editor, true);
2025+
findReplaceHandler->findMatch(editor, 1);
20252026
} else {
2026-
m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, true);
2027+
m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, 1);
20272028
}
20282029
}, [this] {
20292030
if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {
@@ -2039,9 +2040,9 @@ namespace hex::plugin::builtin {
20392040
ContentRegistry::UserInterface::addMenuItem({ "hex.builtin.menu.file", "hex.builtin.view.pattern_editor.menu.find_previous" }, 1530, AllowWhileTyping + SHIFT + Keys::F3, [this] {
20402041
if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {
20412042
ui::TextEditor::FindReplaceHandler *findReplaceHandler = editor->getFindReplaceHandler();
2042-
findReplaceHandler->findMatch(editor, false);
2043+
findReplaceHandler->findMatch(editor, -1);
20432044
} else {
2044-
m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, false);
2045+
m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, -1);
20452046
}
20462047
}, [this] {
20472048
if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {

‎plugins/ui/include/ui/text_editor.hpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ namespace hex::ui {
5959
Coordinates getSelectedColumns();
6060
bool isSingleLine();
6161
bool contains(Coordinates coordinates, int8_t endsInclusive=1);
62+
bool operator==(const Selection &o) const {
63+
return m_start == o.m_start && m_end == o.m_end;
64+
}
65+
bool operator!=(const Selection &o) const {
66+
return m_start != o.m_start || m_end != o.m_end;
67+
}
6268
};
6369

6470
struct EditorState {
@@ -72,7 +78,7 @@ namespace hex::ui {
7278
typedef std::vector<EditorState> Matches;
7379
Matches &getMatches() { return m_matches; }
7480
bool findNext(TextEditor *editor);
75-
u32 findMatch(TextEditor *editor, bool isNex);
81+
u32 findMatch(TextEditor *editor, i32 index);
7682
bool replace(TextEditor *editor, bool right);
7783
bool replaceAll(TextEditor *editor);
7884
std::string &getFindWord() { return m_findWord; }
@@ -687,7 +693,6 @@ namespace hex::ui {
687693
bool m_raiseContextMenu = false;
688694
Coordinates m_focusAtCoords = {};
689695
bool m_updateFocus = false;
690-
bool m_ensureCursorVisible = false;
691696

692697
std::vector<std::string> m_clickableText;
693698

‎plugins/ui/source/ui/text_editor/editor.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,6 @@ namespace hex::ui {
487487
return;
488488

489489
auto pos = setCoordinates(m_state.m_cursorPosition);
490-
//auto start = std::min(pos, m_state.m_selection.m_start);
491490

492491
insertTextAt(pos, value);
493492
m_lines[pos.m_line].m_colorized = false;

‎plugins/ui/source/ui/text_editor/navigate.cpp‎

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ namespace hex::ui {
120120
auto oldPos = m_state.m_cursorPosition;
121121

122122

123-
if (isEmpty() || oldPos.m_line >= (i64)m_lines.size())
123+
if (isEmpty() || oldPos < Coordinates(0, 0))
124124
return;
125125

126126
auto lindex = m_state.m_cursorPosition.m_line;
@@ -163,7 +163,7 @@ namespace hex::ui {
163163

164164
auto oldPos = m_state.m_cursorPosition;
165165

166-
if (isEmpty() || oldPos.m_line >= (i64) m_lines.size())
166+
if (isEmpty() || oldPos > setCoordinates(-1, -1))
167167
return;
168168

169169
auto lindex = m_state.m_cursorPosition.m_line;
@@ -184,11 +184,9 @@ namespace hex::ui {
184184
}
185185

186186
if (select) {
187-
if (oldPos == m_interactiveSelection.m_end) {
188-
m_interactiveSelection.m_end = Coordinates(m_state.m_cursorPosition);
189-
if (m_interactiveSelection.m_end == Invalid)
190-
return;
191-
} else if (oldPos == m_interactiveSelection.m_start)
187+
if (oldPos == m_interactiveSelection.m_end)
188+
m_interactiveSelection.m_end = m_state.m_cursorPosition;
189+
else if (oldPos == m_interactiveSelection.m_start)
192190
m_interactiveSelection.m_start = m_state.m_cursorPosition;
193191
else {
194192
m_interactiveSelection.m_start = oldPos;
@@ -387,16 +385,17 @@ namespace hex::ui {
387385
auto &line = m_lines[at.m_line];
388386
auto charIndex = lineCoordinatesToIndex(at);
389387

390-
if (isWordChar(line.m_chars[charIndex])) {
391-
while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1]))
392-
--charIndex;
393-
} else if (ispunct(line.m_chars[charIndex])) {
394-
while (charIndex > 0 && ispunct(line.m_chars[charIndex - 1]))
395-
--charIndex;
396-
} else if (isspace(line.m_chars[charIndex])) {
397-
while (charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
398-
--charIndex;
388+
bool found = false;
389+
while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1])) {
390+
found = true;
391+
--charIndex;
392+
}
393+
while (!found && charIndex > 0 && ispunct(line.m_chars[charIndex - 1])) {
394+
found = true;
395+
--charIndex;
399396
}
397+
while (!found && charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
398+
--charIndex;
400399
return getCharacterCoordinates(at.m_line, charIndex);
401400
}
402401

@@ -408,16 +407,18 @@ namespace hex::ui {
408407
auto &line = m_lines[at.m_line];
409408
auto charIndex = lineCoordinatesToIndex(at);
410409

411-
if (isWordChar(line.m_chars[charIndex])) {
412-
while (charIndex < (i32) line.m_chars.size() && isWordChar(line.m_chars[charIndex]))
413-
++charIndex;
414-
} else if (ispunct(line.m_chars[charIndex])) {
415-
while (charIndex < (i32) line.m_chars.size() && ispunct(line.m_chars[charIndex]))
416-
++charIndex;
417-
} else if (isspace(line.m_chars[charIndex])) {
418-
while (charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
419-
++charIndex;
410+
bool found = false;
411+
while (charIndex < (i32) line.m_chars.size() && isWordChar(line.m_chars[charIndex])) {
412+
found = true;
413+
++charIndex;
414+
}
415+
while (!found && charIndex < (i32) line.m_chars.size() && ispunct(line.m_chars[charIndex])) {
416+
found = true;
417+
++charIndex;
420418
}
419+
while (!found && charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
420+
++charIndex;
421+
421422
return getCharacterCoordinates(at.m_line, charIndex);
422423
}
423424

@@ -429,17 +430,16 @@ namespace hex::ui {
429430
auto &line = m_lines[at.m_line];
430431
auto charIndex = lineCoordinatesToIndex(at);
431432

432-
if (isspace(line.m_chars[charIndex])) {
433-
while (charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
434-
++charIndex;
435-
}
436-
if (isWordChar(line.m_chars[charIndex])) {
437-
while (charIndex < (i32) line.m_chars.size() && (isWordChar(line.m_chars[charIndex])))
438-
++charIndex;
439-
} else if (ispunct(line.m_chars[charIndex])) {
440-
while (charIndex < (i32) line.m_chars.size() && (ispunct(line.m_chars[charIndex])))
441-
++charIndex;
433+
while (charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
434+
++charIndex;
435+
bool found = false;
436+
while (charIndex < (i32) line.m_chars.size() && (isWordChar(line.m_chars[charIndex]))) {
437+
found = true;
438+
++charIndex;
442439
}
440+
while (!found && charIndex < (i32) line.m_chars.size() && (ispunct(line.m_chars[charIndex])))
441+
++charIndex;
442+
443443
return getCharacterCoordinates(at.m_line, charIndex);
444444
}
445445

@@ -451,17 +451,17 @@ namespace hex::ui {
451451
auto &line = m_lines[at.m_line];
452452
auto charIndex = lineCoordinatesToIndex(at);
453453

454-
if (isspace(line.m_chars[charIndex - 1])) {
455-
while (charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
456-
--charIndex;
457-
}
458-
if (isWordChar(line.m_chars[charIndex - 1])) {
459-
while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1]))
460-
--charIndex;
461-
} else if (ispunct(line.m_chars[charIndex - 1])) {
462-
while (charIndex > 0 && ispunct(line.m_chars[charIndex - 1]))
463-
--charIndex;
454+
bool found = false;
455+
while (charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
456+
--charIndex;
457+
458+
while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1])) {
459+
found = true;
460+
--charIndex;
464461
}
462+
while (!found && charIndex > 0 && ispunct(line.m_chars[charIndex - 1]))
463+
--charIndex;
464+
465465
return getCharacterCoordinates(at.m_line, charIndex);
466466
}
467467

‎plugins/ui/source/ui/text_editor/render.cpp‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ namespace hex::ui {
1919
m_topMarginChanged = true;
2020
}
2121

22-
void TextEditor::setFocusAtCoords(const Coordinates &coords, bool ensureVisible) {
22+
void TextEditor::setFocusAtCoords(const Coordinates &coords, bool scrollToCursor) {
2323
m_focusAtCoords = coords;
2424
m_updateFocus = true;
25-
m_ensureCursorVisible = ensureVisible;
25+
m_scrollToCursor = scrollToCursor;
2626
}
2727

2828
void TextEditor::clearErrorMarkers() {
@@ -199,11 +199,13 @@ namespace hex::ui {
199199
auto height = ImGui::GetWindowHeight() - m_topMargin - scrollBarSize - m_charAdvance.y;
200200
auto width = ImGui::GetWindowWidth() - windowPadding.x - scrollBarSize;
201201

202-
auto top = (i32) rint((m_topMargin > scrollY ? m_topMargin - scrollY : scrollY) / m_charAdvance.y);
203-
auto bottom = top + (i32) rint(height / m_charAdvance.y);
202+
auto topPixels = m_topMargin > scrollY ? m_topMargin - scrollY : scrollY;
203+
auto top = (i32) rint(topPixels / m_charAdvance.y) + 1;
204+
top -= (top >= (i32) m_lines.size());
205+
auto bottom = (i32) rint((topPixels + height) / m_charAdvance.y);
204206

205207
auto left = (i32) rint(scrollX / m_charAdvance.x);
206-
auto right = left + (i32) rint(width / m_charAdvance.x);
208+
auto right = (i32) rint((scrollX + width) / m_charAdvance.x);
207209

208210
auto pos = setCoordinates(m_state.m_cursorPosition);
209211
pos.m_column = (i32) rint(textDistanceToLineStart(pos) / m_charAdvance.x);
@@ -221,16 +223,24 @@ namespace hex::ui {
221223
}
222224

223225
if (mScrollToCursorY) {
224-
if (pos.m_line < top)
225-
ImGui::SetScrollY(std::max(0.0f, pos.m_line * m_charAdvance.y));
226-
if (pos.m_line > bottom)
226+
if (pos.m_line < top) {
227+
ImGui::SetScrollY(std::max(0.0f, (pos.m_line - 1) * m_charAdvance.y));
228+
m_scrollToCursor = true;
229+
}
230+
if (pos.m_line > bottom) {
227231
ImGui::SetScrollY(std::max(0.0f, pos.m_line * m_charAdvance.y - height));
232+
m_scrollToCursor = true;
233+
}
228234
}
229235
if (mScrollToCursorX) {
230-
if (pos.m_column < left)
236+
if (pos.m_column < left) {
231237
ImGui::SetScrollX(std::max(0.0f, pos.m_column * m_charAdvance.x));
232-
if (pos.m_column > right)
238+
m_scrollToCursor = true;
239+
}
240+
if (pos.m_column > right) {
233241
ImGui::SetScrollX(std::max(0.0f, pos.m_column * m_charAdvance.x - width));
242+
m_scrollToCursor = true;
243+
}
234244
}
235245
m_oldTopMargin = m_topMargin;
236246
}
@@ -354,7 +364,7 @@ namespace hex::ui {
354364
void TextEditor::setFocus() {
355365
m_state.m_cursorPosition = m_focusAtCoords;
356366
resetCursorBlinkTime();
357-
if (m_ensureCursorVisible)
367+
if (m_scrollToCursor)
358368
ensureCursorVisible();
359369

360370
if (!this->m_readOnly)
@@ -607,7 +617,7 @@ namespace hex::ui {
607617
ImVec2 TextEditor::calculateCharAdvance() const {
608618
/* Compute mCharAdvance regarding scaled font size (Ctrl + mouse wheel)*/
609619
const float fontSize = ImGui::GetFont()->CalcTextSizeA(ImGui::GetFontSize(), FLT_MAX, -1.0f, "#", nullptr, nullptr).x;
610-
return ImVec2(fontSize, ImGui::GetTextLineHeightWithSpacing() * m_lineSpacing);
620+
return ImVec2(fontSize, GImGui->FontSize * m_lineSpacing);
611621
}
612622

613623
float TextEditor::textDistanceToLineStart(const Coordinates &aFrom) {

0 commit comments

Comments
 (0)