Skip to content

Fix multiline annotations in over- elems in math changing the baseline#5459

Merged
laurmaedje merged 1 commit intotypst:mainfrom
mkorje:underover-multiline-annotations
Dec 8, 2024
Merged

Fix multiline annotations in over- elems in math changing the baseline#5459
laurmaedje merged 1 commit intotypst:mainfrom
mkorje:underover-multiline-annotations

Conversation

@mkorje
Copy link
Collaborator

@mkorje mkorje commented Nov 24, 2024

Fixes #5426.

Note: how vec is layouted when it has elements with linebreaks has changed. Though this change is for the better imo, as it is more consistent with matrices.

Before After
old new
#set page(width: auto, height: auto, margin: 1em)

$ vec(1, c + d \ e + f, 3)
  mat(1; c + d \ e + f; 3) $

@EpicEricEE
Copy link
Contributor

I think there should be some kind of feedback then (e.g. a warning) instead of silently ignoring any line breaks, but maybe that's out of scope here.

@PgBiel
Copy link
Contributor

PgBiel commented Nov 24, 2024

I'm not sure I'm too comfortable with ignoring the linebreaks either. If it's for consistency, maybe the change should happen in the opposite direction (fix linebreaks where they're already broken instead of the other way around).

At the very least, this should be reflected in tests.

@mkorje
Copy link
Collaborator Author

mkorje commented Nov 25, 2024

I agree ignoring linebreaks isn't great (though it seems a little cursed to have a linebreak inside an element in a mat/vec).

Vectors probably shouldn't be using the stack function to be layouted anyway (?). I think we should change vectors to use the same layout method as matrices, which would resolve #5422. I'll have a look at why the linebreaks are being ignored in matrices. Adding these changes to this PR is maybe a bit out of scope though.

@mkorje
Copy link
Collaborator Author

mkorje commented Nov 25, 2024

Ah, I just realised the linebreaks are probably ignored because there would otherwise be issues with alignment points. We wouldn't be able to differentiate alignment points for the whole matrix from ones for the specific element. Maybe, as @EpicEricEE suggested, a warning would be best?

EDIT: Also, the fact that the linebreak works as it does in vec isn't exactly great either. Currently, each line in an element of the vec becomes its own element in the vec when it goes through stack.

@laurmaedje
Copy link
Member

The same happens for cases and I think some people write cases(a \ b \ c) rather than using commas. Not saying it's necessarily great, but I'm uncomfortable with merging this as-is, without unifying vec & mat first and potentially displaying some warnings.

@laurmaedje laurmaedje marked this pull request as draft November 29, 2024 09:29
@mkorje mkorje force-pushed the underover-multiline-annotations branch from 50abd60 to 56a0e31 Compare December 7, 2024 07:25
@mkorje mkorje marked this pull request as ready for review December 7, 2024 07:26
@mkorje
Copy link
Collaborator Author

mkorje commented Dec 7, 2024

I've updated this PR to keep the current behaviour and added tests reflecting the current situation for cases, matrices, and vectors. The linked issue has still been fixed, and this is now the only (user-facing) change made by this PR.

I'll address the inconsistency between vectors (and cases) and matrices with regards to linebreaks when unifying vectors and matrices.

@laurmaedje laurmaedje added this pull request to the merge queue Dec 8, 2024
@laurmaedje
Copy link
Member

Thanks!

Merged via the queue into typst:main with commit 468a601 Dec 8, 2024
@mkorje mkorje deleted the underover-multiline-annotations branch December 9, 2024 01:05
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.

Multi-line overbraces push annotated terms out of alignment with equation

4 participants