Skip to content

markdown: Fix markdown table selection hit testing#47720

Merged
ConradIrwin merged 1 commit intozed-industries:mainfrom
cppcoffee:markdown-table-select
Feb 2, 2026
Merged

markdown: Fix markdown table selection hit testing#47720
ConradIrwin merged 1 commit intozed-industries:mainfrom
cppcoffee:markdown-table-select

Conversation

@cppcoffee
Copy link
Contributor

@cppcoffee cppcoffee commented Jan 27, 2026

Release Notes:

  • Fixed markdown table selection hit testing

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 27, 2026
@cppcoffee
Copy link
Contributor Author

cppcoffee commented Jan 27, 2026

Before:

before.mov

After:

after.mov

@ConradIrwin
Copy link
Member

ConradIrwin commented Jan 27, 2026

Amazing, thank you for the fix!

Is there a way to factor things such that we iterate over the layouts just once? Maybe we keep track of the "fallback line" as we go, but remove the early returns from the loop.

@ConradIrwin ConradIrwin self-assigned this Jan 27, 2026
@cppcoffee cppcoffee force-pushed the markdown-table-select branch from d658fdf to 3b88512 Compare January 27, 2026 06:51
@cppcoffee
Copy link
Contributor Author

cppcoffee commented Jan 27, 2026

Hi @ConradIrwin , refactor done.

ConradIrwin
ConradIrwin reviewed Jan 28, 2026
View reviewed changes
crates/markdown/src/markdown.rs Outdated

continue;
if line_containing_position.is_none() && line_bounds.contains(&position) {
line_containing_position = Some(line);
Copy link
Member

@ConradIrwin ConradIrwin Jan 28, 2026

Choose a reason for hiding this comment

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

@cppcoffee we can just return here, or do you want the last such matching line?

Copy link
Contributor Author

@cppcoffee cppcoffee Jan 28, 2026

Choose a reason for hiding this comment

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

Logic simplification done

@cppcoffee cppcoffee force-pushed the markdown-table-select branch 2 times, most recently from 0d3a749 to e7cc675 Compare January 28, 2026 05:54
@ConradIrwin
Copy link
Member

ConradIrwin commented Jan 30, 2026

Now we're iterating over the list twice again? Maybe I'm missing something...

@cppcoffee
markdown: Fix markdown table selection hit testing
98bd355 
Signed-off-by: Xiaobo Liu <cppcoffee@gmail.com>
@cppcoffee cppcoffee force-pushed the markdown-table-select branch from e7cc675 to 98bd355 Compare January 31, 2026 03:12
@cppcoffee
Copy link
Contributor Author

cppcoffee commented Jan 31, 2026

Now we're iterating over the list twice again? Maybe I'm missing something...

Changed to a single loop

Handle both exact match and Y-coordinate fallback match in a single loop
Use the fallback_line variable to record the first line matching the Y-coordinate

@ConradIrwin
Copy link
Member

ConradIrwin commented Feb 2, 2026

Thanks, and sorry for the back and forth!

@ConradIrwin ConradIrwin enabled auto-merge (squash) February 2, 2026 16:13
@ConradIrwin ConradIrwin merged commit 70c47e9 into zed-industries:main Feb 2, 2026
43 of 45 checks passed
@cppcoffee cppcoffee deleted the markdown-table-select branch February 3, 2026 01:02
adb-sh pushed a commit to adb-sh/zed that referenced this pull request Feb 5, 2026
@cppcoffee @adb-sh
markdown: Fix markdown table selection hit testing (zed-industries#47720
023f636 
)

Release Notes:

- Fixed markdown table selection hit testing

Signed-off-by: Xiaobo Liu <cppcoffee@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ConradIrwin ConradIrwin ConradIrwin left review comments

Assignees

@ConradIrwin ConradIrwin

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cppcoffee @ConradIrwin