lsd-rs / lsd

The next gen ls command
Apache License 2.0
12.8k stars 413 forks source link

Fixed lack of space between executable construction icon and the start of file name #1056

Closed Mersid closed 1 week ago

Mersid commented 1 month ago

Makes executable files go from looking like this: image

To this: image

Notice in the original image there's no spacing between the construction crane icon and the file's name.


TODO

zwpaper commented 4 weeks ago

hi @Mersid, thanks for the PR, but this is related to the icons, can we should not fixed it like this, because when the icon sits with other icons of the same size, it will have extra space.

maybe we should try to detect the width of icons, but it would be much more complicated

Mersid commented 2 weeks ago

Heya @zwpaper! Probably. I can't say for certain what the issue is, but if you try using unicode-width, it appears that the construction icon is marked as having a width of 1, but others like the file and folder icons have a width of 2. However, when rendering, this is obviously not the case, so thus the construction icons are rendered too close together. Just adding the space may be a viable option until we find something better. For now, having some stuff being misaligned does look quite odd. image

Since there's a discrepancy between what the size is stated as, and what it's actually rendered as, the space could simply fix it. I'm not currently aware of other icons that may have this issue.

zwpaper commented 2 weeks ago

Hi @Mersid, thanks for the info on unicode-width, it helps!

it seems reasonable for the default unicode theme to add this space, but we should calculate the width and handle the icon width dynamically.

can you create a issue for use unicode-width to handle this, and add as TODO comment here, so that we can fix this when we have unicode-width

muniu-bot[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Mersid

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment