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

Refactor checks for exa command & the --git option #3

Closed
wants to merge 1 commit into from

Conversation

yhchen0906
Copy link
Contributor

Add a check for the --git option since it might not available in some platform (e.g. aarch64, arm)
See: eza v0.18.11

@ericbn
Copy link
Member

ericbn commented Jun 10, 2024

Hi @yhchen0906. Can we replace the exa --git ${tmp_dir} test by exa --git --help ?

EDIT: I see we cannot. See eza-community/eza#1019

@yhchen0906
Copy link
Contributor Author

Hi @ericbn, I found a way to check the availability of --git option without creating/listing temp directory.
Please take a look

@yhchen0906
Copy link
Contributor Author

Hello @ericbn. I've seen your modifications. Unfortunately, aliases do not take effect within scripts, so your approach means the condition /bin/ls --git will always be evaluated as false. That's why my previous implementation used a function to avoid the issue of aliases not working in scripts.

@ericbn
Copy link
Member

ericbn commented Jun 13, 2024

Oh, you're completely right. I forgot that aliases don't work in the same script after the script has been zcompiled. My bad! Just pushed a commit with the fix.

@yhchen0906
Copy link
Contributor Author

yhchen0906 commented Jun 13, 2024

Hi @ericbn
Perhaps you should modify the exa check as the following

# Ensure exa is available
if (( ${+commands[eza]} )); then
  exa() { eza "$@"; }
elif (( ! ${+commands[exa]} )); then
  return 1
fi

since the original implementation prefers eza over exa

# Ensure exa is available
if (( ${+commands[eza]} )); then
  alias ls='eza --group-directories-first'
elif (( ${+commands[exa]} )); then
  alias ls='exa --group-directories-first'
else
  return 1
fi

Also, make sure to change the list command on line 12 as well :)

EDIT:
I've tested the following code on x86_64 & aarch64 Linux with corresponding eza platform binary
https://github.com/yhchen0906/exa/blob/master/init.zsh

@ericbn
Copy link
Member

ericbn commented Jun 13, 2024

Very good point again! What if we change the first block to:

# Ensure eza is available
if (( ! ${+commands[eza]} )); then
  if (( ${+commands[exa]} )); then
    eza() { exa "$@" }
  else
    return 1
  fi
fi

?

You're right about prefering eza over exa (in case both are available). I don't want to create a exa function if eza is installed though, since most users should be using eza by now and exa is deprecated.

@yhchen0906
Copy link
Contributor Author

I see. In that case, this implementation does indeed meet expectations better. Thank you very much!

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

Successfully merging this pull request may close these issues.

2 participants