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

Weird grid alignment #145

Closed
petobens opened this issue Mar 7, 2019 · 12 comments · Fixed by #151
Closed

Weird grid alignment #145

petobens opened this issue Mar 7, 2019 · 12 comments · Fixed by #151
Labels
kind/bug Something isn't working

Comments

@petobens
Copy link

petobens commented Mar 7, 2019

Hi! I've started using lsd today (great project BTW :) and noticed some strange alignment:
screenshot_2019-03-07_05 42 21

Is that a bug or a feature? I'm on arch using alacritty. Thanks in advance!

BTW: is possible to add custom icons? for instance I would like to have an icon for symlinked folders, R extension, etc. I can open a new issue for this

@petobens petobens added the kind/bug Something isn't working label Mar 7, 2019
@Peltoche
Copy link
Collaborator

Peltoche commented Mar 7, 2019

Hi @petobens ,

Thanks for the report! This is absolutely not a feature... Can you give us the output of lsd --version please?

At the moment their is no way to customize the icons only for your binary but you can propose your icons to everyone by doing a PR, the R extension would be welcomed at least.

@meain
Copy link
Member

meain commented Mar 7, 2019

I had hit this at times. I did some investigation and it feels like a bug in unicode width computation in term_grid. But, I have not had enough time to go behind that.

@Peltoche
Copy link
Collaborator

Peltoche commented Mar 7, 2019

It's curious because it doesn't seem to be related to a specific icon or a specific color and those are the two possible source of errors for the string length calculation used by term_grid.

Without a reproducible case, this will be complicated to fix.

@meain
Copy link
Member

meain commented Mar 7, 2019

@petobens do you have any complex lscolors (.dircolors) file defined? If so, could you maybe post it here?

@jonathanstrong
Copy link

I just ran into this, and disabling some ls dircolors stuff in my .bashrc fixed it:

# enable color support of ls and also add handy aliases
if [ -x /usr/bin/dircolors ]; then
    test -r ~/.dircolors && eval "$(dircolors -b ~/.dircolors)" || eval "$(dircolors -b)"
    alias ls='ls --color=auto'
    #alias dir='dir --color=auto'
    #alias vdir='vdir --color=auto'

    alias grep='grep --color=auto'
    alias fgrep='fgrep --color=auto'
    alias egrep='egrep --color=auto'
fi

I actually don't even know what dircolors is, just thought this would help.

@petobens
Copy link
Author

petobens commented Mar 8, 2019

Thanks for the quick reply. AFAICT I only have this

> echo $LS_COLORS
di=0;34:ln=0;35:ex=0;31:

I'm using lsd version 0.13.0

At the moment their is no way to customize the icons only for your binary but you can propose your icons to everyone by doing a PR, the R extension would be welcomed at least.

Are there plans to add a config file to customize such icons and also colors in general (beyond LS_COLORS)?

@petobens
Copy link
Author

petobens commented Mar 8, 2019

Actually when I running dircolors I get:

> dircolors
LS_COLORS='rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.zst=01;31:*.tzst=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.wim=01;31:*.swm=01;31:*.dwm=01;31:*.esd=01;31:*.jpg=01;35:*.jpeg=01;35:*.mjpg=01;35:*.mjpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:';
export LS_COLORS

This was referenced Mar 8, 2019
@Peltoche
Copy link
Collaborator

Are there plans to add a config file to customize such icons and also colors in general (beyond LS_COLORS)?

Yep, there is some ideas about it (#92) but nobody is working on it at the moment.

@Peltoche
Copy link
Collaborator

I think the error is related to the way we calculate the visible width. It expect the same colors format but with the LS_COLORS support the users can apply RGB colors which doesn't have the same format.

We must be able to handle all these formats and today it's not the case.

The faulty code:

fn get_visible_width(input: &str) -> usize {
    let mut nb_invisible_char = 0;

    for (idx, _) in input.match_indices("\u{1b}[38;5;" /* "\e[38;5;" */) {
        let color_code = input.chars().skip(idx + 7);
        let mut code_size = 0;
        color_code
            .skip_while(|x| {
                code_size += 1;
                char::is_numeric(*x)
            })
            .count();
        nb_invisible_char += 6 + code_size; /* "\e[38;5;" + color number + "m" */
    }

    if nb_invisible_char > 0 {
        // If no color have been set, the is no reset character.
        nb_invisible_char += 3; /* "[0m" */
    }

    UnicodeWidthStr::width(input) - nb_invisible_char
}

@Peltoche
Copy link
Collaborator

I have a possible patch (#151) but it needs some tests. Is it possible that someone with the bug try it locally in order to check if it work? I will try to write some regression tests when I have the time

@meain
Copy link
Member

meain commented Mar 11, 2019

@Peltoche Looks like the issue has been fixed with #151

@petobens
Copy link
Author

Thank you!

Yep, there is some ideas about it (#92) but nobody is working on it at the moment.

Great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants