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

O_NERD and O_EMOJI creates duplicate letters in filename #1692

Closed
7 of 9 tasks
FelixKratz opened this issue Jul 14, 2023 · 15 comments · Fixed by #1711
Closed
7 of 9 tasks

O_NERD and O_EMOJI creates duplicate letters in filename #1692

FelixKratz opened this issue Jul 14, 2023 · 15 comments · Fixed by #1711
Labels

Comments

@FelixKratz
Copy link

FelixKratz commented Jul 14, 2023

Environment details (Put x in the checkbox along with the information)

  • Operating System: macOS Ventura
  • Desktop Environment: Aqua
  • Terminal Emulator: kitty, Alacritty
  • Shell: zsh
  • Custom desktop opener (if applicable):
  • Program options used: none
  • Configuration options set: O_NERD
  • Plugins are installed
  • Issue exists on nnn master

Exact steps to reproduce the issue

Clone the repo, enable nerdfonts in the makefile

O_NERD := 1  # support icons-nerdfont

and compile with LDLIBS="-L/opt/homebrew/opt/ncurses/lib/" make.

Some files (with certain icons it appears) have duplicate initial letters as seen in the screenshot below:
Screenshot 2023-07-14 at 11 48 58

as soon as I select the makefile it will also have the duplicate initial letter:
Screenshot 2023-07-14 at 11 54 34

I have done some testing, this seems to also happen with O_EMOJI is set to enabled:
Screenshot 2023-07-14 at 11 56 51

but not with O_ICONS enabled:
Screenshot 2023-07-14 at 11 57 37

I have peeked into the source code to find what is different between those options, but did not find a convincing clue.

I have also checked with kitty and alacritty, they both face the same problem.

These are the dynamic libs used by nnn:

 x otool -L ./nnn
./nnn:
	/usr/lib/libedit.3.dylib (compatibility version 2.0.0, current version 3.0.0)
	/opt/homebrew/opt/ncurses/lib/libncursesw.6.dylib (compatibility version 6.0.0, current version 6.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.0.0)
@FelixKratz FelixKratz added the bug label Jul 14, 2023
@N-R-K
Copy link
Collaborator

N-R-K commented Jul 14, 2023

Previous discussion: #1662

According to #1662 (reply in thread) v4.7 didn't have this problem (can you confirm this too? @FelixKratz) but I went through the diff between 4.7 and 4.8 and didn't notice anything that might cause it.

Also another person who reported the issue was on macos as well (#1662 (reply in thread)) so maybe something in macos is causing it?

@FelixKratz
Copy link
Author

FelixKratz commented Jul 14, 2023

Sorry, I did not search the discussion, only the issues.

I have done a quick bisect and this is the commit where it goes wrong: 20e944f

This commit changed the codepoints, how would such a change lead to the duplication of filname letters?

@FelixKratz
Copy link
Author

FelixKratz commented Jul 14, 2023

Interestingly, O_EMOJI shows this problem since the introduction in b45be9b.

Edit: The problem appears when calling addstr(icon.icon);.

All instances where this is a problem wcswidth reports a length of 2 as seen in the screenshot below, where I display the wcswidth of the icon to the left of the actual item:
Screenshot 2023-07-14 at 15 08 29
It seems that this messes up some internal counting logic.

@jarun
Copy link
Owner

jarun commented Jul 15, 2023

I use emojis on xfce4-terminal and don't see this issue.
Can you try to fix this and raise a PR? I will try on my setup and see if it works for me as well?
The change should be in print_icon() and/or adjust_cols(). See if changing the padding works.

@FelixKratz
Copy link
Author

FelixKratz commented Jul 15, 2023

I believe this problem has no trivial fix.

It seems that removing

	if (sel)
		attrs |= A_REVERSE;

in printent fixes the duplication problem. This suggests to me that there is a problem with the ncurses internal redraw and damage logic. This is possibly connected to a disagreement in the terminals perceived width for the character and the ncurses calculated width of the character. But to be honest, I have no clue about terminal rendering.

The problematic new nerdfont chars seem to be 4byte wide, while they where 3bytes before the update.
The same is true for the emojis.

@jarun
Copy link
Owner

jarun commented Jul 15, 2023

@luukvbaal would you able to check this?

@jarun
Copy link
Owner

jarun commented Jul 15, 2023

@FelixKratz what happens if you replace A_REVERSE with A_STANDOUT?

@FelixKratz
Copy link
Author

@FelixKratz what happens if you replace A_REVERSE with A_STANDOUT?

As soon as any attrs are set that would cause the filename to be redrawn it starts to happen

No attrs:
Screenshot 2023-07-16 at 15 49 44

A_STANDOUT:
Screenshot 2023-07-16 at 15 50 34

A_BOLD:
Screenshot 2023-07-16 at 15 50 55

@jarun
Copy link
Owner

jarun commented Jul 16, 2023

@N-R-K @luukvbaal what's the best course of action here?

We can't make the next release without fixing this.

I see something here that I don't fully understand: #1661

@N-R-K
Copy link
Collaborator

N-R-K commented Jul 16, 2023

All instances where this is a problem wcswidth reports a length of 2 as seen in the screenshot below, where I display the wcswidth of the icon to the left of the actual item: Screenshot 2023-07-14 at 15 08 29

Interesting. This does seem to be what's causing the issue, because on my system (linux/glibc), all those icons have a width of 1 when called in wcswidth.

image

Could this be a libc bug in macos? Can you share the output of locale? Also what do you see when you run LC_ALL="C.UTF8" nnn?

@FelixKratz
Copy link
Author

Could this be a libc bug in macos? Can you share the output of locale? Also what do you see when you run LC_ALL="C.UTF8" nnn?

My locale output is:
Screenshot 2023-07-16 at 21 46 06

Running LC_ALL="C.UTF8" ./nnn looks completely wild:
Screenshot 2023-07-16 at 21 51 43

I will happily try out any ideas, I am a bit lost regarding this problem as I have no experience with ncurses.

@lawnowner
Copy link
Contributor

lawnowner commented Jul 16, 2023

I don't think your problem is due to a bug in nnn. Unless you can reproduce this on Linux, maybe your OS is using a different text normalization for unicode encoding (e.g. NFD instead of NFC), so the width of some characters are 2 instead of 1 for you due to decomposition.

The locale category values in double quotes are not set explicitly, they are implied values derived from LANG or LC_ALL, but in your output all values are in double quotes, so none of them are explicitly set in your environment. You can try LC_CTYPE="en_US.UTF-8" ./nnn or LC_ALL="en_US.UTF-8" ./nnn on the command line, or setting the LC_ variables in your shell startup file and/or terminal emulator config. If you got a weird utf-8-mac variant of the locale, you can try that as well. Also see setlocale has no effect on macOS.

@cyaconi
Copy link

cyaconi commented Jul 18, 2023

I just tried both LC_CTYPE="en_US.UTF-8" ./nnn and LC_ALL="en_US.UTF-8" ./nnn on macOS and the problem continues. Hope it helps.

@FelixKratz
Copy link
Author

I seem to have found a fix for this: #1694

@quantonganh
Copy link
Contributor

I confirmed that wcswidth returns the different value for the same Unicode code point between macOS and Linux.

macOS:

LANG="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_CTYPE="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_ALL=
Screen Shot 2023-08-07 at 11 25 54

Linux:

LANG=en_GB.UTF-8
LANGUAGE=
LC_CTYPE="en_GB.UTF-8"
LC_NUMERIC="en_GB.UTF-8"
LC_TIME="en_GB.UTF-8"
LC_COLLATE="en_GB.UTF-8"
LC_MONETARY="en_GB.UTF-8"
LC_MESSAGES="en_GB.UTF-8"
LC_PAPER="en_GB.UTF-8"
LC_NAME="en_GB.UTF-8"
LC_ADDRESS="en_GB.UTF-8"
LC_TELEPHONE="en_GB.UTF-8"
LC_MEASUREMENT="en_GB.UTF-8"
LC_IDENTIFICATION="en_GB.UTF-8"
LC_ALL=
Screen Shot 2023-08-07 at 11 26 23

In version 4.8, the code point is different, width is 1 but the icon is missing:

Screen Shot 2023-08-07 at 11 47 57

I don't think your problem is due to a bug in nnn. Unless you can reproduce this on Linux, maybe your OS is using a different text normalization for unicode encoding (e.g. NFD instead of NFC), so the width of some characters are 2 instead of 1 for you due to decomposition.

It could be the result of a combination of nnn and the Nerd fonts, locale settings, unicode version and the C library implementation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants