Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions devel/204_30.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# 204_30 Mismatched bracket sizes in multi-line formulas

## How to test
- Open Mogan editor
- Insert a display formula, use the align environment
- On the first line, enter content with `\left[` (e.g. `f(x) \leq \left[ \int dx`)
- On the second line, enter content with `\right]` (e.g. `+x \right]`)
- Verify that the bracket sizes on both lines are consistent

## 2026/03/06 Fix mismatched bracket sizes in multi-line formulas

### What
Fixed an issue where brackets (e.g. `[` and `]`) spanning multiple lines in multi-line math formulas (such as align environments) had inconsistent sizes.

### Why
Multi-line math environments are internally implemented as tables, with each row typeset as an independent cell. Each cell's bracket size was calculated based only on that row's content, making it impossible to coordinate bracket sizes across rows. For example, the first row with an integral symbol made `[` large, but the shorter content on the second row kept `]` small.

### How
Introduced a bracket-pending mechanism using environment variables, storing bracket height information per column (`math-bracket-pending-{col}`) and propagating it between table rows:

- When a row ends with an incomplete bracket pair (e.g. `<left-[>...<right-.>`), save the vertical extents of that row's content
- When the next row detects an incomplete bracket pair at the start (e.g. `<left-.>...<right-]>`), read the saved extents and merge them
- Supports brackets spanning three or more rows, with middle rows both reading and propagating pending state

Modified files:
- `src/Typeset/Concat/concat_post.cpp`: added bracket_match_state, pending state management functions, modified handle_matching and handle_brackets
- `src/Typeset/Concat/concater.hpp`: updated handle_matching function signature
136 changes: 133 additions & 3 deletions src/Typeset/Concat/concat_post.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,84 @@ concater_rep::clean_and_correct () {
* Resize brackets
******************************************************************************/

struct bracket_match_state {
bool left_seen;
bool right_seen;
bool start_empty;
bool end_empty;
bracket_match_state ()
: left_seen (false), right_seen (false), start_empty (false),
end_empty (false) {}
};

static bool
is_empty_left_delimiter (const string& s) {
return starts (s, "<left-.");
}

static bool
is_empty_right_delimiter (const string& s) {
return starts (s, "<right-.");
}

static bracket_match_state
classify_bracket_match (array<line_item>& a, int start, int end) {
bracket_match_state st;
for (int i= start; i <= end; i++) {
int tp= a[i]->type;
if (tp == LEFT_BRACKET_ITEM) {
st.left_seen= true;
if (i == start)
st.start_empty= is_empty_left_delimiter (a[i]->b->get_leaf_string ());
}
else if (tp == RIGHT_BRACKET_ITEM) {
st.right_seen= true;
if (i == end)
st.end_empty= is_empty_right_delimiter (a[i]->b->get_leaf_string ());
}
}
return st;
}

static string
bracket_pending_key (edit_env env) {
tree row= env->read (CELL_ROW_NR);
tree col= env->read (CELL_COL_NR);
if (!is_atomic (row) || !is_int (row)) return "";
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The variable row at line 340 is read from the environment but is only used for validation (to check we're inside a table cell). It is not used in constructing the pending key. This will likely produce a compiler warning about an unused variable. Consider either using (void) row; to suppress the warning, replacing the named variable with a direct is-validation expression, or explicitly marking the intention in a comment.

Suggested change
if (!is_atomic (row) || !is_int (row)) return "";
if (!is_atomic (row) || !is_int (row)) return "";
// 'row' is read to ensure we are inside a table cell; its value is not
// needed for constructing the pending key.
(void) row;

Copilot uses AI. Check for mistakes.
if (!is_atomic (col) || !is_int (col)) return "";

string key= "math-bracket-pending";
key << "-" << col->label;
return key;
}

static bool
get_bracket_pending (edit_env env, const string& key, SI& y1, SI& y2) {
if (N (key) == 0) return false;
tree pending= env->read (key);
if (!is_tuple (pending) || N (pending) != 2) return false;
if (!is_int (pending[0]) || !is_int (pending[1])) return false;
y1= as_int (pending[0]);
y2= as_int (pending[1]);
return true;
}

static void
set_bracket_pending (edit_env env, const string& key, SI y1, SI y2) {
if (N (key) == 0) return;
env->write (key, tree (TUPLE, as_string ((int) y1), as_string ((int) y2)));
}

static void
clear_bracket_pending (edit_env env, const string& key) {
if (N (key) == 0) return;
env->write (key, tree (TUPLE));
}

void
concater_rep::handle_matching (int start, int end) {
concater_rep::handle_matching (int start, int end, bool use_pending,
SI pending_y1, SI pending_y2, SI& out_y1,
SI& out_y2) {
// cout << "matching " << start << " -- " << end << "\n";
// cout << a << "\n\n";
int i;
Expand All @@ -320,6 +396,12 @@ concater_rep::handle_matching (int start, int end) {
y1= min (a[start]->b->y1, a[end]->b->y2);
y2= max (a[start]->b->y1, a[end]->b->y2);
}
if (use_pending) {
y1= min (y1, pending_y1);
y2= max (y2, pending_y2);
}
out_y1= y1;
out_y2= y2;

for (i= start; i <= end; i++) {
int tp= a[i]->type;
Expand Down Expand Up @@ -407,25 +489,73 @@ concater_rep::handle_matching (int start, int end) {

void
concater_rep::handle_brackets () {
string pending_key= bracket_pending_key (env);
SI pending_y1 = 0;
SI pending_y2 = 0;
bool has_pending=
get_bracket_pending (env, pending_key, pending_y1, pending_y2);
bool pending_was_present= has_pending;
bool pending_touched = false;

int first= -1, start= 0, i= 0;
while (i < N (a)) {
if (a[i]->type == LEFT_BRACKET_ITEM) {
if (first == -1) first= i;
start= i;
}
if (a[i]->type == RIGHT_BRACKET_ITEM) {
bracket_match_state st= classify_bracket_match (a, start, i);
bool use_pending=
has_pending && (st.start_empty || (!st.left_seen && st.right_seen));
SI match_y1= 0, match_y2= 0;
handle_scripts (succ (start), prec (i));
handle_matching (start, i);
handle_matching (start, i, use_pending, pending_y1, pending_y2, match_y1,
match_y2);
if (st.end_empty || (st.left_seen && !st.right_seen)) {
pending_y1 = match_y1;
pending_y2 = match_y2;
has_pending = true;
pending_touched= true;
}
else if ((st.start_empty && !st.end_empty) ||
(!st.left_seen && st.right_seen)) {
has_pending = false;
pending_touched= true;
}
if (first != -1) i= first - 1;
start= 0;
first= -1;
}
i++;
}
if (N (a) > 0) {
bracket_match_state st= classify_bracket_match (a, 0, N (a) - 1);
bool use_pending=
has_pending && (st.start_empty || (!st.left_seen && st.right_seen));
SI match_y1= 0, match_y2= 0;
handle_scripts (0, N (a) - 1);
handle_matching (0, N (a) - 1);
handle_matching (0, N (a) - 1, use_pending, pending_y1, pending_y2,
match_y1, match_y2);
if (st.end_empty || (st.left_seen && !st.right_seen)) {
pending_y1 = match_y1;
pending_y2 = match_y2;
has_pending = true;
pending_touched= true;
}
else if ((st.start_empty && !st.end_empty) ||
(!st.left_seen && st.right_seen)) {
has_pending = false;
pending_touched= true;
}
}

Comment on lines +509 to +551
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The pending-state update logic in lines 509-526 (inside the while loop) and lines 534-551 (the final block) are nearly identical, duplicating about 12 lines of complex conditional logic. This creates a maintenance burden since both blocks must be updated together if the logic changes. Consider extracting these repeated blocks into a shared helper function to improve maintainability.

Copilot uses AI. Check for mistakes.
if (has_pending && pending_was_present && !pending_touched)
has_pending= false;

if (has_pending)
Comment on lines +554 to +555
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The guard at line 554–555 clears the pending state for any row that had a pending bracket from a previous row but didn't touch the pending state (no bracket items encountered). This prevents stale state from leaking across unrelated rows, but it also prevents the pending state from being propagated through middle rows that happen to have no bracket items at all.

For example, in a 3-row align environment:

  • Row 1: f(x) \leq \left[ \int dx \right. → sets pending ✓
  • Row 2: + x (no brackets) → pending is cleared by line 554–555 ✗
  • Row 3: \right] → never sees the pending, sizes incorrectly ✗

The PR description claims "Supports brackets spanning three or more rows", but this only works when every middle row also contains bracket pair items (which is typically the case when TeXmacs automatically inserts implicit \left. and \right. markers). If a user explicitly writes a middle row with no implicit bracket markers, the fix breaks down. A comment noting this limitation would be helpful.

Copilot uses AI. Check for mistakes.
set_bracket_pending (env, pending_key, pending_y1, pending_y2);
else if (pending_was_present || pending_touched)
clear_bracket_pending (env, pending_key);
}

/******************************************************************************
Expand Down
3 changes: 2 additions & 1 deletion src/Typeset/Concat/concater.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ class concater_rep {
void glue (box b, int ref, int arg1, int arg2);
void clean_and_correct ();
void handle_scripts (int start, int end);
void handle_matching (int start, int end);
void handle_matching (int start, int end, bool use_pending, SI pending_y1,
SI pending_y2, SI& out_y1, SI& out_y2);
void handle_brackets ();
void kill_spaces ();

Expand Down