zed icon indicating copy to clipboard operation
zed copied to clipboard

Lines greater than 1024 chars long crash the editor on Windows 11.

Open professorbee opened this issue 9 months ago • 6 comments

Check for existing issues

  • [X] Completed

Describe the bug / provide steps to reproduce it

Lines greater than 1024 characters break windows. Steps to reproduce:

  1. Create a file in another text editor with a line containing 1025 characters.
  2. Open in editor.
  3. Watch it crash.

Here's the file I used: test.txt

Environment

Zed: v0.135.0 (Zed Dev d64106e01b0ebec37d00ff0fd28f6848e16a997b) OS: Windows 10.0.22631 Memory: 31.7 GiB Architecture: x86_64

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

Zed.log

professorbee avatar May 07 '24 21:05 professorbee

i can confirm that this issue also occurs for me on the latest commit: 6a64b73. OS: Windows 11 22631.2861 Memory: 32GB Arch: x86_64 Zed.log i created a new text file in Zed and copy-pasted line containing 1025 characters from your test.txt - crashed immediately, file was not saved. exit code: 0xffffffff

pidjan avatar May 07 '24 21:05 pidjan

It seems to break (but not crash) on Linux as well. You can not scroll past 1024 horizontally and if you insert anything it simply goes off the visible area.

someone13574 avatar May 08 '24 02:05 someone13574

Found the problem. I used git bisect to find that https://github.com/zed-industries/zed/pull/11080 (this commit specifically) changes the behavior of the soft-wrap mode "none" to not wrap for very long lines, which doesn't respect MAX_LINE_LEN (which is set to 1024), causing a crash (or clipping if on linux and mac) when the line length exceeds that instead of wrapping at 512. Temporarily, you can fix this by adding "soft_wrap": "prefer_line" to your settings.json. I'm not sure how @SomeoneToIgnore want's to deal with it long term if no wrapping is required for git hunks but setting “prefer line” should work for now.

This long line wrapping seems like it was added for this exact reason (fixing clipping) in https://github.com/zed-industries/zed/pull/866 which was a response to https://github.com/zed-industries/zed/issues/834

someone13574 avatar May 08 '24 03:05 someone13574

PreferLine variant is supposed to be the new default, as the one being called None before — the only place current None is intended to be used is git diff hunks.

So, I see two problems with Windows impl:

  • usage of None instead of PreferLine
  • panics on long lines

I would not want to deal with Windows anyhow special on this regard, IMO it's supposed to work with the long lines too disregarding of the wrapping mode.

SomeoneToIgnore avatar May 08 '24 06:05 SomeoneToIgnore

This surprised me. I tested it, and when calling layout_line, the input text parameter has only 1024 characters, but the FontRun parameter indicates there are 1025 characters, causing a panic when accessing text[index]. I think the issue lies in the input parameter when calling layout_line, but it can also be easily fixed in DirectWrite. Should I fix it on the DirectWrite side or where layout_line is called?

JunkuiZhang avatar May 08 '24 07:05 JunkuiZhang

#11536 should fix this, I tested it on my laptop.

JunkuiZhang avatar May 08 '24 08:05 JunkuiZhang