Homebrew / brew

🍺 The missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
41.33k stars 9.72k forks source link

zsh-completion error : showing `usage: date [-jnu] ...` #5839

Closed zijuexiansheng closed 5 years ago

zijuexiansheng commented 5 years ago

Please note that we will close your issue without comment if you delete, do not read or do not fill out the issue checklist below and provide ALL the requested information. If you repeatedly fail to use the issue template, we will block you from ever submitting issues to Homebrew again.

What you were trying to do (and why)

I'm trying to use the autocompletion of homebrew and got some error output.

What happened (include command output)

When I hit the tab to auto complete, I constantly get the following output.

Command output

usage: date [-jnu] [-d dst] [-r seconds] [-t west] [-v[+|-]val[ymwdHMS]] ... 
            [-f fmt date | [[[mm]dd]HH]MM[[cc]yy][.ss]] [+format]
usage: date [-jnu] [-d dst] [-r seconds] [-t west] [-v[+|-]val[ymwdHMS]] ... 
            [-f fmt date | [[[mm]dd]HH]MM[[cc]yy][.ss]] [+format]
usage: date [-jnu] [-d dst] [-r seconds] [-t west] [-v[+|-]val[ymwdHMS]] ... 
            [-f fmt date | [[[mm]dd]HH]MM[[cc]yy][.ss]] [+format]

  

Issue description and proposed solution

This error will not appear in Linux system (or maybe newer Mac OS). My system is Mac OS 10.10.5 and the date command is different than the one in the Linux system.

This issue is caused by the date -r "$1" +s command in the __brew_completion_caching_policy() function in the _brew file. By the detailed output I posted above, the -r parameter should be followed by "seconds" rather than a file name. So I propose to use stat -f "%m" "$1" instead. One can define the following function

function __loon_last_mod_time() {
    if [[ "$(uname 2> /dev/null)" == "Linux" ]]; then
        date -r "$1" +%s
    else
        stat -f "%m" "$1"
    fi
}

and replace the __brew_completion_caching_policy() by

__brew_completion_caching_policy() {
  # rebuild cache if no cache file exists (anyway we cannot proceed further down)
  ! [[ -f "$1" ]] && return 0
  # cache file modification date (seconds since epoch)
  local -i cache_mtime=$(__get_last_mod_time "$1")  #<--- this line
  # latest modified homebrew tap index file
  local latest_modified_git_index=(${HOMEBREW_REPOSITORY:-/usr/local/Homebrew}/Library/Taps/*/*/.git/index(om[1]))
  local -i commit_mtime=$(__get_last_mod_time "$latest_modified_git_index")  #<--- this line
  (( $cache_mtime < $commit_mtime ))
}
okdana commented 5 years ago

Yeah that function is really not ideal. It's certainly unnecessary to involve external utility calls just to check file mtimes. Off the top of my head i would suggest something like this:

__brew_completion_caching_policy() {
  local -a tmp

  # Invalidate if cache file is >2 weeks old
  tmp=( $1(mw-2N) )
  (( $#tmp )) || return 0

  # Otherwise, invalidate if latest tap index file is newer than cache file
  tmp=( ${HOMEBREW_REPOSITORY:-/usr/local/Homebrew}/Library/Taps/*/*/.git/index(om[1]N) )
  [[ $tmp -nt $1 ]]
}

On a related note, the command helper functions shouldn't keep forcibly setting a cache-policy like they're doing. The main _brew function should set it once and only if it's not already set. (The reason the caching system uses the zstyle mechanism at all is to allow the user to customise how it works. Forcibly overriding it every time defeats that purpose.)

I just noticed this ticket at the top of the list when i was going to search for another issue, so i don't have a PR prepared, but if nobody else beats me to it maybe i can do something later.

MikeMcQuaid commented 5 years ago

This issue is caused by the date -r "$1" +s command in the __brew_completion_caching_policy() function in the _brew file. By the detailed output I posted above, the -r parameter should be followed by "seconds" rather than a file name. So I propose to use stat -f "%m" "$1" instead. One can define the following function

This seems reasonable. Can you (or @okdana) open a PR? Thanks!