Skip to content

Conversation

@jackmath5261-bit
Copy link
Contributor

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements an initial version of a floating text selection toolbar for TeXmacs, similar to modern text editors. When text is selected, a toolbar appears above the selection with formatting buttons (bold, italic, underline, highlight, color) and alignment options (left, center, right).

Changes:

  • Adds new QTMTextToolbar widget class implementing the floating toolbar UI with Qt
  • Integrates toolbar display logic into the editor interface to show/hide based on text selection state
  • Updates CSS themes (liii and liii-night) and resource files to support toolbar styling and icons

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
src/Plugins/Qt/qt_simple_widget.hpp Adds forward declaration and method declarations for text toolbar support
src/Plugins/Qt/qt_simple_widget.cpp Implements text toolbar lifecycle methods (create, show, hide, scroll, hit test)
src/Plugins/Qt/QTMTextToolbar.hpp Defines the QTMTextToolbar widget class with button members and positioning cache
src/Plugins/Qt/QTMTextToolbar.cpp Implements the toolbar widget with 8 formatting/alignment buttons and positioning logic
src/Edit/Interface/edit_mouse.cpp Adds mouse event handling to show/hide toolbar and implements helper methods for selection detection
src/Edit/Interface/edit_keyboard.cpp Triggers toolbar update after keyboard events
src/Edit/Interface/edit_interface.hpp Declares text toolbar interface methods
src/Edit/Interface/edit_interface.cpp Calls toolbar update when selection changes
devel/201_63.md Provides testing instructions for the feature
TeXmacs/misc/themes/liii.css Adds styling for the text toolbar in light theme
TeXmacs/misc/themes/liii-night.css Adds styling for the text toolbar in dark theme
TeXmacs/misc/images/images.qrc Adds toolbar icon resources (with some duplicates)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

get_canvas_x (), get_canvas_y ());
}
else {
// 即使矩形无效,也尝试显示工具栏(例如单个字符选区)
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "即使矩形无效,也尝试显示工具栏(例如单个字符选区)" (Even if the rectangle is invalid, try to show the toolbar (e.g., single character selection)), but the code actually hides the toolbar when the rectangle is invalid. This is a contradiction - the comment should be corrected to match the actual behavior.

Suggested change
// 即使矩形无效,也尝试显示工具栏(例如单个字符选区
// 如果矩形无效,则隐藏工具栏(例如单个字符选区尚未形成有效矩形时

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +34
<file>ocr-button/left-align.svg</file>
<file>ocr-button/middle-align.svg</file>
<file>ocr-button/right-align.svg</file>

<!-- dark theme text toolbar icons -->
<file>../pixmaps/liii-night/20x20/mode/tm_bold.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_italic.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_underline.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_marked.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_color.svg</file>
<file>ocr-button/left-align-white.svg</file>
<file>ocr-button/middle-align-white.svg</file>
<file>ocr-button/right-align-white.svg</file>

Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These file entries are duplicates of lines 3-5. The same files should not be listed multiple times in the resource file.

Suggested change
<file>ocr-button/left-align.svg</file>
<file>ocr-button/middle-align.svg</file>
<file>ocr-button/right-align.svg</file>
<!-- dark theme text toolbar icons -->
<file>../pixmaps/liii-night/20x20/mode/tm_bold.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_italic.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_underline.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_marked.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_color.svg</file>
<file>ocr-button/left-align-white.svg</file>
<file>ocr-button/middle-align-white.svg</file>
<file>ocr-button/right-align-white.svg</file>
<!-- dark theme text toolbar icons -->
<file>../pixmaps/liii-night/20x20/mode/tm_bold.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_italic.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_underline.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_marked.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_color.svg</file>

Copilot uses AI. Check for mistakes.
Comment on lines +896 to +899
QToolButton#text-toolbar-button[icon-name="align-justify"] {
qproperty-icon: url(":/text-toolbar/align-justify.svg");
}

Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CSS references an icon ":/text-toolbar/align-justify.svg" that is not included in the images.qrc resource file and there's no corresponding button in QTMTextToolbar.cpp. Either this icon should be added to images.qrc if it's intended for future use, or this CSS rule should be removed if the align-justify button is not planned.

Suggested change
QToolButton#text-toolbar-button[icon-name="align-justify"] {
qproperty-icon: url(":/text-toolbar/align-justify.svg");
}

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +181
cachePosition (selr, magf, scroll_x, scroll_y, canvas_x, canvas_y);
updatePosition (ren);
show ();
raise ();
}
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The autoSize method should be called before calculating the toolbar position in showTextToolbar, similar to how it's done in QTMImagePopup::showImagePopup (line 118 in QTMImagePopup.cpp). Currently, autoSize is only called in the constructor when cached_magf is still 0.0, which means the button sizes will be calculated incorrectly. This will lead to incorrect toolbar dimensions being used for positioning.

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +40
bool painted;
int painted_count;
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The member variables 'painted' and 'painted_count' are initialized but never used. In QTMImagePopup, these are used to delay showing the popup until it has been painted twice (see QTMImagePopup.cpp lines 116-129). Either these variables should be removed if the delay mechanism is not needed for the text toolbar, or the same show-delay logic should be implemented for consistency.

Suggested change
bool painted;
int painted_count;
[[maybe_unused]] bool painted;
[[maybe_unused]] int painted_count;

Copilot uses AI. Check for mistakes.
Comment on lines +1044 to +1071
rectangle
edit_interface_rep::get_text_selection_rect () {
rectangle sel_rect;

if (selection_active_any () && !is_nil (selection_rects)) {
// 使用现有的选区矩形
sel_rect= least_upper_bound (selection_rects);
}
else if (selection_active_any ()) {
// 如果没有选区矩形,但选区存在,计算一个默认矩形
path p1, p2;
selection_get (p1, p2);
if (p1 != p2) {
selection sel= search_selection (p1, p2);
if (!is_nil (sel->rs)) {
sel_rect= least_upper_bound (sel->rs);
}
else {
// 如果选区矩形为空,使用光标位置创建一个最小矩形
cursor cu= get_cursor ();
sel_rect = rectangle (cu->ox - 10 * pixel, cu->oy - 5 * pixel,
cu->ox + 10 * pixel, cu->oy + 5 * pixel);
}
}
}

return sel_rect;
}
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function can return an uninitialized rectangle if selection_active_any() returns false, or if selection_active_any() is true but selection_rects is nil and p1 == p2. This uninitialized rectangle could lead to undefined behavior when accessed in update_text_toolbar. The function should either initialize sel_rect with a default value at declaration or ensure all code paths properly initialize it.

Copilot uses AI. Check for mistakes.

/*文本工具栏窗口样式*/

/*文本工具栏窗口样式*/
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate comment. This line is identical to line 854.

Suggested change
/*文本工具栏窗口样式*/

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +34
<file>ocr-button/left-align.svg</file>
<file>ocr-button/middle-align.svg</file>
<file>ocr-button/right-align.svg</file>

<!-- dark theme text toolbar icons -->
<file>../pixmaps/liii-night/20x20/mode/tm_bold.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_italic.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_underline.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_marked.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_color.svg</file>
<file>ocr-button/left-align-white.svg</file>
<file>ocr-button/middle-align-white.svg</file>
<file>ocr-button/right-align-white.svg</file>

Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These file entries are duplicates of lines 10-12. The same files should not be listed multiple times in the resource file.

Suggested change
<file>ocr-button/left-align.svg</file>
<file>ocr-button/middle-align.svg</file>
<file>ocr-button/right-align.svg</file>
<!-- dark theme text toolbar icons -->
<file>../pixmaps/liii-night/20x20/mode/tm_bold.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_italic.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_underline.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_marked.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_color.svg</file>
<file>ocr-button/left-align-white.svg</file>
<file>ocr-button/middle-align-white.svg</file>
<file>ocr-button/right-align-white.svg</file>
<!-- dark theme text toolbar icons -->
<file>../pixmaps/liii-night/20x20/mode/tm_bold.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_italic.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_underline.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_marked.svg</file>
<file>../pixmaps/liii-night/20x20/mode/tm_color.svg</file>

Copilot uses AI. Check for mistakes.
Comment on lines +153 to +162
[=] (QAbstractButton* button) {
if (this->owner) {
if (button == alignLeftBtn) {
call ("make-line-with", object ("par-mode"), object ("left"));
}
else if (button == alignCenterBtn) {
call ("make-line-with", object ("par-mode"),
object ("center"));
}
else if (button == alignRightBtn) {
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda capture uses [=] (capture by value) which is inconsistent with the other lambdas in this file that use [this]. For consistency and clarity, this should be changed to [this]. Additionally, capturing by value is potentially dangerous here because it captures the button pointers (alignLeftBtn, alignCenterBtn, alignRightBtn) by value, which could become dangling pointers if the buttons are deleted.

Suggested change
[=] (QAbstractButton* button) {
if (this->owner) {
if (button == alignLeftBtn) {
call ("make-line-with", object ("par-mode"), object ("left"));
}
else if (button == alignCenterBtn) {
call ("make-line-with", object ("par-mode"),
object ("center"));
}
else if (button == alignRightBtn) {
[this] (QAbstractButton* button) {
if (this->owner) {
if (button == this->alignLeftBtn) {
call ("make-line-with", object ("par-mode"), object ("left"));
}
else if (button == this->alignCenterBtn) {
call ("make-line-with", object ("par-mode"),
object ("center"));
}
else if (button == this->alignRightBtn) {

Copilot uses AI. Check for mistakes.
Comment on lines 192 to 193
cached_scroll_x+= x;
cached_scroll_y+= y;
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scrollBy implementation differs from QTMImagePopup's implementation. In QTMImagePopup, scrollBy subtracts the scroll values after dividing by cached_magf (lines 148-149 in QTMImagePopup.cpp). However, this implementation directly adds the raw scroll values without dividing by cached_magf. This inconsistency will cause the text toolbar to scroll at a different rate than the image popup, leading to incorrect positioning behavior during scrolling.

Suggested change
cached_scroll_x+= x;
cached_scroll_y+= y;
if (cached_magf != 0.0) {
cached_scroll_x-= static_cast<int> (std::round (x / cached_magf));
cached_scroll_y-= static_cast<int> (std::round (y / cached_magf));
}
else {
cached_scroll_x-= x;
cached_scroll_y-= y;
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants