Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change how the width of the gutter is calculated considering propotional fonts. #21860

Open
1 task done
matj1 opened this issue Dec 11, 2024 · 3 comments · May be fixed by #24009
Open
1 task done

Change how the width of the gutter is calculated considering propotional fonts. #21860

matj1 opened this issue Dec 11, 2024 · 3 comments · May be fixed by #24009
Labels
design papercut Small visual defect editor Feedback for code editing, formatting, editor iterations, etc good first issue Issue suitable for first-time contributors

Comments

@matj1
Copy link

matj1 commented Dec 11, 2024

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

Pull request #4135 sets that there are always 4 ems of space reserved for the line number to prevent changing the width much. The idea is reasonable, but em is defined as the width of ‘m’. In proportional fonts, ‘m’ is usually much wider than numerals, so the gutter is too wide. That can be seen in this comment.

The same em is used to calculate padding, so the gutter also has too much padding.

Reproduction

  1. Set the buffer font to something proportional (like Noto Sans): in settings "buffer_font_family": "Noto Sans"
  2. See that the gutter is too wide.

Solution

I suggest that the width of the numeral ‘0’ would be used as the width of a digit in the gutter. The width of digits usually does not vary much, and, if so, usually just because ‘1’ is narrower. So there is practically no risk of not enough space in the gutter for 4 digits.

It seems that it would be solved by redefining the variables em_width and em_advance in the appropriate functions. When #4135 was merged, the appropriate functions were compute_layout and compute_auto_height_layout in crates/editor/src/element.rs. But the file has changed, so I don't know which are the appropriate functions now.

The variables would be redefined so:

let em_width = cx
    .text_system()
    .typographic_bounds(font_id, font_size, '0') // not 'm'
    .unwrap()
    .size
    .width;
let em_advance = cx
    .text_system()
    .advance(font_id, font_size, '0') // not 'm'
    .unwrap()
    .width;

The names no longer reflect their function, so they should be changed to something more suitable.

Environment

Zed: v0.164.2 (Zed)
OS: Linux X11 arch unknown
Memory: 15.6 GiB
Architecture: x86_64
GPU: AMD Radeon RX 580 2048SP (RADV POLARIS10) || radv || Mesa 24.3.1-arch1.1

If applicable, add mockups / screenshots to help explain present your vision of the feature

a mockup showing how wide the number column is and how wide it should be

@matj1 matj1 added admin read bug [core label] labels Dec 11, 2024
@JosephTLyons JosephTLyons added editor Feedback for code editing, formatting, editor iterations, etc design papercut Small visual defect and removed triage labels Dec 20, 2024
@iamnbutler iamnbutler added good first issue Issue suitable for first-time contributors and removed bug [core label] labels Jan 30, 2025
@iamnbutler
Copy link
Member

I agree, that seems like a resoable solution.

I added the good first issue tag, as the fix should be reasonably straightforward and would be a great way for new contributors to jump in and contribute.

Otherwise someone can feel free to jump in and fix this.

@TimmyMcPickles
Copy link

I'd like to try and check this issue out, if that would be alright

TimmyMcPickles added a commit to TimmyMcPickles/zed that referenced this issue Jan 31, 2025
…lement.rs lines 6403, 6485, and 6491 from an 'm' to '0'
@flvrone
Copy link
Contributor

flvrone commented Jan 31, 2025

@iamnbutler, hey, in that case - would you consider looking at the tiny changes I have made in my local PR?
flvrone#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design papercut Small visual defect editor Feedback for code editing, formatting, editor iterations, etc good first issue Issue suitable for first-time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants