devblackops / Terminal-Icons

A PowerShell module to show file and folder icons in the terminal
MIT License
2.32k stars 104 forks source link

Alignment issues in some hosts with Format-Wide (May be Nerd Fonts V3 specific) #109

Open JK-Flip-Flop96 opened 1 year ago

JK-Flip-Flop96 commented 1 year ago

Expected Behavior

Get-ChildItem | Format-Wide should produce output in aligned columns

Current Behavior

Screenshot 2023-06-06 100449 "ls" in the above screenshot is aliased to a function that ultimately outputs Get-ChildItem | Format-Wide -AutoSize with some arguments passed to GCI. I've tested with the exact command too. ]

Doesn't affect long listing (plain Get-Childitem) or a custom Tree-like function (that invokes Format-TerminalIcon) which I wrote for my profile.

(Ignore my attempts at obscuring my employer in the above, realised it was futile when it's literally in my profile bio 😆)

Possible Solution

I suspect this is something to do with the rendering of wide glyphs by the terminal. But I've been unable to narrow it down to a specific subset of glyphs.

Steps to Reproduce (for bugs)

  1. Install a build of the NF v3 compatible HEAD
  2. Open Powershell in one of the hosts mentioned below
  3. Import the built Module
  4. gci | fw

Context

Tested Environments

Windows Terminal.

It appears to work in 1.16 but doesn't work in 1.17 and 1.18 Preview (Shown above).

conhost.exe (pwsh.exe)

image

Closer to working correctly, some entries appear with a 3 wide gap between the icon and the name.

Alacritty

image Works as intended

Other observations

Outputs are consistent, you always get the same icons offset by the same amount given, provided the setup is the same (host/font/font size/number of columns/screen width).

I also tested in Hyper.js with an NF v2 font with an NF v3 theme still set and the results were similar to WT. (Couldn't be bothered to update the font on a terminal app I don't use)

Your Environment

Apologies for having so many variables in my testing, I just tried everything I had on hand out of interest 😅.

Please let me know if this issue should be passed on to the Microsoft/terminal repo or elsewhere.

MattKotsenas commented 1 month ago

I only just started to look into this, but I'd assume this is because of surrogate pairs (I'm not great at fonts, so I might be getting terms wrong).

It appears that if an icon uses the material design font section, its represented as two UTF-16 code points. That's then causing a miscalculation of string length.

Here's a snippet of a potential workaround:

diff --git a/Terminal-Icons/Private/Resolve-Icon.ps1 b/Terminal-Icons/Private/Resolve-Icon.ps1
index 3c2c071..58e0e40 100644
--- a/Terminal-Icons/Private/Resolve-Icon.ps1
+++ b/Terminal-Icons/Private/Resolve-Icon.ps1
@@ -127,6 +127,11 @@ function Resolve-Icon {
             $displayInfo['Icon'] = $null
         }
         $displayInfo['Color'] = $colorSeq
+
+        #$Host.UI.WriteLine("Before: $($displayInfo['Icon']) $($displayInfo['Icon'].Length)")
+        if ($displayInfo['Icon'].Length -eq 1) { $displayInfo['Icon'] = ([char]0x00).ToString() + $displayInfo['Icon']  }
+        #$Host.UI.WriteLine(" After: $($displayInfo['Icon']) $($displayInfo['Icon'].Length)")
+
         $displayInfo
     }
 }

Here at the end of Resolve-Icon, I check to see if the icon is a single code point. If so, I pad it with a NUL character. I wouldn't recommend this as the actual solution, just a proof-of-concept.

MattKotsenas commented 1 month ago

I filed https://github.com/PowerShell/PowerShell/issues/23861 as I think this is also an upstream issue in PowerShell.

That said we probably should work around the issue here as well, given that this is unlikely to be a quick fix.

MattKotsenas commented 1 month ago

This feels like something that could be fixed easily without performance impact after #120 is in, as the list could be processed at build time and add the appropriate padding.

@devblackops I'm happy to do the work here, but the approach will depend on the outcome of 120. Let me know how best to proceed.