Fix #1404, and more...

[zhelezov]

This was supposed to be just a fix for #1404 but upon visiting the source
several issues became apparent and that is why the commit grew a bit more than
expected. A complete list of changes bellow:

• Fix _HLEDGER_COMPLETION_TEMPDIR creates an additional directory on each shell invocation #1404
  No more orphaned temporary directories. Commands, options, etc. that used to be
  stored in there are included at build-time as here documents in the source.

• Fix artifacts in /tmp after build
  Upon fixing the above I became aware that the build itself was leaving behind a
  heap of artifacts in /tmp that were not taken care of with a make clean.
  Fixed by using temporary files and directories in the build directory. Makefile
  and build scripts adjusted.

• Produce command aliases
  Regular expressions in build scripts changed to produce all command aliases
  except single letter ones (see below)

• Do not propose single letters completions
  It is simply not useful and adds a lot of noise. It makes completion slower as
  well because you need to hit yes on the prompt:
  Display all 200 possibilities? (y or n)
output-options.sh now excludes those.

• Query filters simplified
  Keep only the prefix of the filter with the colon in query-filters.txt. This
  change has two reasons:

  • Single letter completions are not useful (see above)
  • It allows for completion suggestions specific to each
  • Bonus reason: it's a completion engine, not a user manual.

• Fix completion impacts on global environment
  The completion script was making a couple of changes to the global environment
  which had an impact for the rest of the shell session.

  • set -o pipefail: the change is hidden from the user and could lead to subtle
    errors throughout the shell session
  • COMP_WORDBREAKS=" ": this affects subsequent completions for us and other
    programs too. I exclude the colon : from its value and use
    compopt -o filenames to handle escaping of special characters for us. I would
    like to find a solution without messing with COMP_WORDBREAKS but it is not
    straight forward.

• Fix hiding of legit subcommands
  Completion was hiding all possibilities if a subcommand happens to be the prefix
  of another. On typing balance, one should be proposed balancesheet and
  balancesheetequity as well.

• Return early
  Try to complete depending on the current context and return immediately if
  successful. Keep completion list relevant and as short as possible.

• Context aware completion

  • Add handlers for option parameter completion, see _hledger_compreply_optarg()
  • Add handlers for query filters:, see _hledger_compreply_query()
  • Use --file and --rules-file arguments when proposing completions for the
    above, see _hledger()
  • Propose only top level accounts at first. Again, keep it short and focused.

• Custom compgen wrapper
  compgen is fairly complicated. There is no way to feed it a word list with
  literals. It will mangle your input in so many ways that we cannot trust it. To
  work around this several wrappers are used: _hledger_compgen() works with
  _hledger_quote_by_ref() to process and escape newline separated input which is
  then fed to compgen and finally in COMPREPLY through _hledger_compreply()
  and _hledger_compreply_append(). It sounds messy and I guess it is, I would like
  to find a more straight forward way to do it. I think it is still a way better
  and safer interface with readline than trying to grep our way through.

• Replace declare with local
  Again, this script is sourced by the shell -- keep variable scopes as narrow as
  possible. Oops, they are actually synonymous when used in a function but
  local declares our intentions explicitly.

• Use compopt -o nosort
  Often I resort to using it to keep different groups of completions together.
  Whether this is more ergonomic or not is subjective. But our input lists are
  already sorted at build-time so why not. Sort manually query-filters.txt when
  editing it.

• Remove irrelevant comments
  And add some new ones :)

I think that is all. Give it a spin, try to abuse it, in and outside of quotes,
with some funky accounts, payees, tags, whatever, and tell me where it breaks or
behaves unexpectedly.

[zhelezov]

If you miss the completion for the query filters that I have deleted from query-filters.txt, like amt:>, amt:<=, etc., I could add them back in the case statement in _hledger_compreply_query(). E.g.:

    case $query in
        acct:)  hledgerArgs=(accounts --flat) ;;
        code:)  hledgerArgs=(codes) ;;
        cur:)   hledgerArgs=(commodities) ;;
        desc:)  hledgerArgs=(descriptions) ;;
        note:)  hledgerArgs=(notes) ;;
        payee:) hledgerArgs=(payees) ;;
        tag:)   hledgerArgs=(tags) ;;
        *)
            local wordlist
            case $query in
                amt:)    wordlist="< <= > >=" ;;
                real:)   wordlist="\  0" ;;
                status:) wordlist="\  * !" ;;
                *)       return 1 ;;
            esac
            _get_comp_words_by_ref -n '<=>' -c wordToComplete
            _hledger_compreply "$(compgen -P "$query" -W "$wordlist" -- "${wordToComplete#*:}")"
            return 0
            ;;
    esac

I am not convinced of its usefulness but just saying that they could be added back easily if deemed necessary.

[zhelezov]

Also, what about this loop:

hledger/shell-completion/hledger-completion.bash

Lines 276 to 296 in 4d2253b

	for (( i=1; i < ${#COMP_WORDS[@]} - 2; i++ )); do
	case ${COMP_WORDS[i]} in
	-f|--file)
	if [[ ${COMP_WORDS[i+1]} == '=' ]]; then
	hledgerFile=${COMP_WORDS[i+2]}
	else
	hledgerFile=${COMP_WORDS[i+1]}
	fi
	# Pass it through compgen to unescape it
	hledgerFile=$(compgen -W "$hledgerFile")
	;;
	--rules-file)
	if [[ ${COMP_WORDS[i+1]} == '=' ]]; then
	hledgerRulesFile=${COMP_WORDS[i+2]}
	else
	hledgerRulesFile=${COMP_WORDS[i+1]}
	fi
	hledgerRulesFile=$(compgen -W "$hledgerRulesFile")
	;;
	esac
	done

Should I avoid the code duplication? Is the alternative better, clear enough I mean:

    for (( i=1; i < ${#COMP_WORDS[@]} - 2; i++ )); do
        case ${COMP_WORDS[i]} in
            # Use $file as a name reference
            -f|--file)
                local -n file=hledgerFile ;;
            --rules-file)
                local -n file=hledgerRulesFile ;;
            *)
                continue ;;
        esac
        if [[ ${COMP_WORDS[i+1]} == '=' ]]; then
            file=${COMP_WORDS[i+2]}
        else
            file=${COMP_WORDS[i+1]}
        fi
        # Pass it through compgen to unescape it
        file=$(compgen -W "$file")
    done

Or, define a general function that can be reused to fetch any option argument from the command line:

_hledger_optarg() {
    local options=("$@")
    local optarg i j offset

    for (( i=1; i < ${#COMP_WORDS[@]} - 2; i++ )); do
        offset=0
        for j in "${!options[@]}"; do
            if [[ ${COMP_WORDS[i]} == "${options[j]}" ]]; then
                if [[ ${COMP_WORDS[i+1]} == '=' ]]; then
                    offset=2
                else
                    offset=1
                fi
                # Pass it through compgen to unescape it
                optarg=$(compgen -W "${COMP_WORDS[i + offset]}")
            fi
        done
        ((i += offset))
    done

    printf %s "$optarg"
}

_hledger() {
    local hledgerArgs=("$@")
    local hledgerFile
    local hledgerRulesFile

    hledgerFile=$(_hledger_optarg -f --file)
    hledgerRulesFile=$(_hledger_optarg --rules-file)

    [[ -f $hledgerFile ]] && hledgerArgs+=(--file "$hledgerFile")
    [[ -f $hledgerRulesFile ]] && hledgerArgs+=(--rules-file "$hledgerRulesFile")

    hledger "${hledgerArgs[@]}" 2>/dev/null
}

[zhelezov]

Oops, is this a bug or a feature? If I pass two files to hledger it combines them instead of using the last one as I supposed:

hledger -f journal-1 -f journal-2 balance

If that is the expected behaviour I need to adjust the above to pass all files to the hledger call, not only the last one. What about the --rules-file? Should I use all or only the last one?

[simonmichael]

Thanks for this very comprehensive patch! It sounds good, I hope @schoettl will comment in a bit.

Yes multiple input files are possible with multiple -f's. A single --rules-file overrides the rules for all csv files, so I think only the last --rules-file is significant.

[schoettl]

Thanks for mentioning me. I'll definitely review until weekend.

[schoettl]

Hey, I like your clean code and nice patches. I want to try it out also in the next days.

[schoettl]

Query filters simplified
Keep only the prefix of the filter with the colon in query-filters.txt. This
change has two reasons:

* Single letter completions are not useful (see above)

* It allows for completion suggestions specific to each

* Bonus reason: it's a completion engine, not a user manual.

... but did I see it right in the code, that you still have context aware completions for e.g. amt:>=?

I see your last point a bit differently. Sure, it's completion engine, not manual. But if the completions – at no costs – help the users to learn new features and remind them how these are spelled, I think this is a very good thing. Including single-letter like ! and * in status.

OK, but without tried it yet, if I understand your change in the m4 file correctly, you have considered this in the context-aware completion?

[zhelezov]

Yep, that point in the first commit is no longer relevant because I've added them back in 6b35938. I thought as well that it doesn't cost us anything to offer them, so here they are again. But we don't need them in query-filters.txt -- >, >=, *, !, etc. are hardcoded completions for the respective filter, amt: and status: in this case. For complete breakdown of query filters handling see here:

hledger/shell-completion/hledger-completion.bash

Lines 224 to 259 in 6b35938

	_hledger_compreply_query() {
	[[ $wordToComplete =~ .: ]] || return
	local query=${wordToComplete%%:*}:
	grep -Fxqe "$query" <<< "$_hledger_complist_query_filters" || return
	local hledgerArgs=()
	case $query in
	acct:) hledgerArgs=(accounts --flat) ;;
	code:) hledgerArgs=(codes) ;;
	cur:) hledgerArgs=(commodities) ;;
	desc:) hledgerArgs=(descriptions) ;;
	note:) hledgerArgs=(notes) ;;
	payee:) hledgerArgs=(payees) ;;
	tag:) hledgerArgs=(tags) ;;
	*)
	local wordlist
	case $query in
	amt:) wordlist="< <= > >=" ;;
	real:) wordlist="\ 0" ;;
	status:) wordlist="\ * !" ;;
	*) return 1 ;;
	esac
	_get_comp_words_by_ref -n '<=>' -c wordToComplete
	_hledger_compreply "$(compgen -P "$query" -W "$wordlist" -- "${wordToComplete#*:}")"
	return 0
	;;
	esac
	_hledger_compreply "$(
	_hledger_compgen "$(
	_hledger "${hledgerArgs[@]}" | sed "s/^/$query/g"
	)"
	)"
	return 0
	}

[zhelezov]

And thank you for the review @schoettl!

[zhelezov]

And a hint for testing -- do not test in a shell which has sourced the original completion first, the environment will be compromised and some very fun things to debug may happen.

[zhelezov]

Edit: Forget it, it doesn't take into account any input inserted by the user without the help of completion. Too good to be true!

An idea: what if we take a more rigid stance and offer subcommand completion only as the second argument. We can get rid of that whole loop in the main function and reduce it to:

    if ((cword == 1)); then
        _hledger_compreply "$(_hledger_compgen "$_hledger_complist_commands")"
        return 0
    else
        subcommand=${words[1]}
        subcommandOptions=_hledger_complist_options_${subcommand//-/_}
        _hledger_compreply "$(_hledger_compgen "${!subcommandOptions}")"
    fi

And the whole function becomes:

_hledger_completion_function() {
    # Current treatment for special characters:
    # - exclude colon (:) from COMP_WORDBREAKS
    # - use comptop -o filenames to escape the rest
    COMP_WORDBREAKS=${COMP_WORDBREAKS//:}

    local cur prev words cword
    _init_completion -n : || return 0

    local subcommand subcommandOptions
    if ((cword == 1)); then
        _hledger_compreply "$(_hledger_compgen "$_hledger_complist_commands")"
        return 0
    else
        subcommand=${words[1]}
        subcommandOptions=_hledger_complist_options_${subcommand//-/_}
        _hledger_compreply "$(_hledger_compgen "${!subcommandOptions}")"
    fi

    [[ $cur == -* ]] && return

    # Most option arguments, query filters and accounts need to be escaped
    # so it is easier to set this here and unset it only in specific cases
    compopt -o filenames

    # Option argument completion
    _hledger_compreply_optarg && return

    # Query completion
    _hledger_compreply_query && return

    # Subcommand specific
    case $subcommand in
        files|test) return 0 ;;
        help)
            compopt -o nosort +o filenames
            _hledger_compreply_append "$(compgen -W "$(hledger help | tail -n 1)" -- "$cur")"
            return 0
            ;;
    esac

    # Offer query filters and accounts for the rest
    # Do not sort. Keep options, accounts and query filters grouped separately
    compopt -o nosort -o nospace
    _hledger_compreply_append "$(_hledger_compgen "$_hledger_complist_query_filters")"
    if [[ -z $cur ]]; then
        _hledger_compreply_append "$(_hledger_compgen "$(_hledger accounts --flat --depth 1)")"
    else
        _hledger_compreply_append "$(_hledger_compgen "$(_hledger accounts --flat)")"
    fi

    return 0
}

Liberty traded for efficiency. Thoughts on this?

[simonmichael]

Sorry I don't have more feedback.. I never use shell completions, because I'm usually in an emacs shell where they don't work (I believe). I must give the latest a try in a terminal..

When you two feel this is ready for merging, let me know.

[zhelezov]

Hi @simonmichael, I would like some more time to optimize it as much as possible and to see the suggestions of @schoettl. No hurry. And I also use it mostly in Emacs with a ton of premade queries but now and then when you need to run that odd one... And with the vterm module for Emacs I've got a fully-fledged shell, yey!

[simonmichael]

Cool, I will have to look into vterm..

[alerque]

I'm a pretty ugly hack when it comes to Haskell but I do know my way around shell code, M4, GNU Make, and friends. I haven't compiled and given this a test drive but I did take a few minutes to read through the diff and the code looks great. Hats off to you for the attention to detail, very legible coding style, and proper handling of things people often get wrong in these contexts. I won't suggest it is perfect or bug free but what I did see looked very good and I'm looking forward to in landing. Thanks for taking the time to contribute to this project.

[zhelezov]

Thanks for the kind words @alerque. I am not a frequent contributor, and I am afraid I don't have the experience to do it properly. My excuses to @schoettl for piling up too many changes at once to review. I am open to critique so my next contribution won't suffer from my ignorance. Do give it a spin if you find the time though. I would like some more input on the ergonomics and of course... bugs!

Cheers

[alerque]

It might be a good idea to check the shell scripts in shellcheck (or even automatically fix them up with shellharden). Style wise it doesn't always do things the way I would prefer and I sometimes fiddle with it afterwards –for example combining quoted segments– but its awareness of where there could be quoting or scoping issues is really solid.

[zhelezov]

I have yet to look into shellharden, but I do use shellcheck together with Emacs' flycheck and I've already fixed the few warnings it spit out a few commits ago. Thanks for the suggestions, I'll give it a go.

Edit: And done... Nothing to worry about. Found a bug in shellharden though -- it doesn't seem to get right nested case statements.

[zhelezov]

This can be simplified even further like this:

            subcommand=
            break

...and sub-command completion will be offered next, but feels more vague, and means a few more instructions.

[zhelezov]

On extraction of commands, options, etc.

It's all too easy to break a leg dancing with the regex. There should be a way to extract all commands, aliases, and the options they support using the Hledger.Cli.CliOptions module. Options (aka flags) seem to be grouped under several groups: helpflags, inputflags, reportflags, hiddenflags, etc., and if I get this right every command is assigned one or more of these groups, plus any options specific to that command only. So there should be a way to stitch together an ugly hack that produces an easily parsable output that suits our needs for every possible hledgerCommandMode instead of relying on sed to parse free flow text meant for human consumption. Any pointers how do I go about pursuing that? Could it be implemented as a test that is called interactively with hledger test?

[simonmichael]

@zhelezov yes, options/flags are grouped that way so that they can be reused consistently by hledger's various tools and commands.

I believe https://hackage.haskell.org/package/cmdargs has something built in or an add-on that exposes the structure so you could consume it programmatically, eg there is https://hackage.haskell.org/package/cmdargs-browser that can generate a ui for any cmdargs-using program. This sort of thing will take some research. (If it were me, I would save that for a separate PR.)

[simonmichael]

PS, I might be able to give more ideas on the hledger chat.

[zhelezov]

@simonmichael Yeah, I will look into your suggestions on another rainy day and leave it for another PR. I will pop in #hledger to discuss this further then.

@schoettl I was thinking that we should probably rename hledger-completion.bash to hledger.bash because bash completion's dynamic loading machinery assumes that completion definitions are stored in a file with the comand's name. In our case that would be hledger, hledger.bash or _hledger. And it adds one more step for the user to install this properly, namely to rename the file correctly. I just added an example installation comment and thought about this.

And a question -- looking at the Makefile, I see that hledger-completion.bash is trashed by the clean target. Why is it this way if it is supposed to be in the repo? Is it just to make sure that it is rebuild properly? In that case it can be added to the PHONY targets and left out from the clean one.

And one more -- should we add an install target?

[zhelezov]

@alerque Thanks again for streamlining the build process. When you've got a minute I want to summon your make-fu again to check my last additions to the Makefile.

[zhelezov]

Is there something similar to the BSD m4's paste(file) in GNU m4, i.e. to include a file without any macro expansion?

[simonmichael]

Seems like great work ongoing here. When it comes to fancy make/m4, I'll just note our highest priority is robustness, portability and minimizing roadbumps and costs for installers/contributors/maintainers.

[simonmichael]

PS so that's a good question you're asking. Don't know the answer, sorry!

[zhelezov]

Seems like great work ongoing here. When it comes to fancy make/m4, I'll just note our highest priority is robustness, portability and minimizing roadbumps and costs for installers/contributors/maintainers.

That's exactly my reason for asking because most of the included files are meant to be included literally, so it would be nice if there was a way to prevent accidental macro expansion or quoting issues, so future contributors aren't burdened with that. It's not that of an issue but something one has to keep in mind.

[zhelezov]

I see I can easily extract supported checks for completion with something like this:

hledger check --help | sed -rn 's/^-\s+([a-z-]+)\s+-.*/\1/p'

But I don't know how long that output will remain parsable, so tell me if you want that. Or simply hard-code them, as another possibility. Example patch:

diff --git a/shell-completion/hledger-completion.bash.stub b/shell-completion/hledger-completion.bash.stub
index 2c4862615..08f490ba3 100644
--- a/shell-completion/hledger-completion.bash.stub
+++ b/shell-completion/hledger-completion.bash.stub
@@ -102,8 +102,18 @@ _hledger_completion() {
 
     # Subcommand specific
     case $subcommand in
+        check)
+            compopt +o filenames
+            _hledger_compreply "$(
+                _hledger_compgen "$(
+                    hledger check --help |
+                        sed -rn 's/^-[[:space:]]+([a-z][a-z-]+)[[:space:]]+-.*/\1/p'
+                )"
+            )"
+            return 0
+            ;;
         # These do not expect or support any query arguments
-        commodities|check|files|help|import|print-unique|test)
+        commodities|files|help|import|print-unique|test)
             return 0
             ;;
     esac

Or rather do this at build time and include specific command arguments as heredocs, as we already do for options.

[zhelezov]

About --info and --man. After some investigation, it's only the addons that are wrongly listing them in their help.

hledger ui --info
hledger web --man

Those are noops.

[simonmichael]

Thanks @zhelezov. We should probably add check --list. But the current names should be quite stable, so I think it'd be fine to hard-code them for this release.

Those are noops.

Good to know, thanks.

[simonmichael]

I applied the hard-coding patch. However, I'm seeing an error when testing setup:

$ make install
Makefile:32: *** Error running ./parse-commands.sh.  Stop.

Probably because I'm on a mac.

[simonmichael]

.SHELLSTATUS requires GNU Make 4.2+, while mac has only GNU Make 3.x. I commented out this error check as it didn't seem critical.

I'm still not able to test this on mac though, see below.

16:09:16 ~/src/hledger/shell-completion$ make install
install: hledger-completion.bash -> /Users/simon/.local/share/bash-completion/completions/hledger
symlink /Users/simon/.local/share/bash-completion/completions/hledger-ui -> hledger
symlink /Users/simon/.local/share/bash-completion/completions/hledger-web -> hledger
16:09:19 ~/src/hledger/shell-completion$ hledger<TAB>
.gitignore                    foreach2.m4                   parse-commands.sh
BSDmakefile                   hledger-completion.bash       parse-options.sh
Makefile                      hledger-completion.bash.m4    query-filters.txt
README.md                     hledger-completion.bash.stub  quote.m4
16:09:19 ~/src/hledger/shell-completion$ source /Users/simon/.local/share/bash-completion/completions

16:09:43 ~/src/hledger/shell-completion$ hledger
hledger                           hledger-get-currency-prices.sh
hledger-0.22.2                    hledger-iadd
hledger-0.23.2                    hledger-import-shared-expenses
hledger-0.26                      hledger-install.sh
hledger-0.27                      hledger-interest
hledger-1.0.1                     hledger-print-location
hledger-1.1                       hledger-print-location.hs
hledger-1.10                      hledger-smooth
hledger-1.11                      hledger-smooth.hs
hledger-1.12                      hledger-stockquotes
hledger-1.14                      hledger-swap-dates
hledger-1.15                      hledger-swap-dates.hs
hledger-1.15.2                    hledger-ui
hledger-1.16.1                    hledger-ui-1.10.1
hledger-1.16.2                    hledger-ui-1.11.1
hledger-1.17                      hledger-ui-1.12
hledger-1.17.1.1                  hledger-ui-1.15
hledger-1.18                      hledger-ui-1.16.1
hledger-1.18-146-gf518da74        hledger-ui-1.16.2
hledger-1.18-148-g00d08f8d        hledger-ui-1.17
hledger-1.19                      hledger-ui-1.18
hledger-1.19.1                    hledger-ui-1.19
hledger-1.2                       hledger-ui-1.19.1
--More--
16:09:43 ~/src/hledger/shell-completion$ hledger pr<TAB>bash: _init_completion: command not found
16:09:43 ~/src/hledger/shell-completion$ type __load_completion
-bash: type: __load_completion: not found
16:10:29 ~/src/hledger/shell-completion$ echo $SHELL
/bin/bash
16:10:32 ~/src/hledger/shell-completion$ /bin/bash --version
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin20)
Copyright (C) 2007 Free Software Foundation, Inc.
16:10:35 ~/src/hledger/shell-completion$ cp hledger-completion.bash ~/.bash_completion.d/hledger
cp: /Users/simon/.bash_completion.d/hledger: No such file or directory
16:10:56 ~/src/hledger/shell-completion$ mkdir ~/.bash_completion.d/
16:11:03 ~/src/hledger/shell-completion$ cp hledger-completion.bash ~/.bash_completion.d/hledger
16:11:04 ~/src/hledger/shell-completion$ source ~/.bash_completion.d/hledger
16:11:17 ~/src/hledger/shell-completion$ bash
16:11:22 ~/src/hledger/shell-completion$ complete -p hledger
bash: complete: hledger: no completion specification

[zhelezov]

Hi, I don't have access to a mac system and have not done any testing on one. bash v3.2 is ancient (released in 2006) but even so it should work. Are you sure you have bash-completion loaded? It seems to need some manual work on macOS to make it work: https://github.com/scop/bash-completion#macos-os-x. I will look into this next weekend, see if install locations should be different from those on Linux and BSD.

Can you give this a try? First, make sure bash-completion is present:

brew install bash-completion

Then source it manually from, say .bashrc:

if [ -f $(brew --prefix)/etc/bash_completion ]; then
  . $(brew --prefix)/etc/bash_completion
fi

[simonmichael]

Sorry @zhelezov, I'm not having any luck with this. I did brew install bash-completion and followed its instruction:

Add the following line to your ~/.bash_profile:
  [[ -r "/opt/homebrew/etc/profile.d/bash_completion.sh" ]] && . "/opt/homebrew/etc/profile.d/bash_completion.sh"

and restarted the shell, and to make sure, ran

. /opt/homebrew/etc/profile.d/bash_completion.sh

Then I follow your install steps

$ make install
install: hledger-completion.bash -> /Users/simon/.local/share/bash-completion/completions/hledger
symlink /Users/simon/.local/share/bash-completion/completions/hledger-ui -> hledger
symlink /Users/simon/.local/share/bash-completion/completions/hledger-web -> hledger

and to make sure, ran

. /Users/simon/.local/share/bash-completion/completions/hledger

but still get

hledger pr<TAB>-bash: _init_completion: command not found

Maybe some other mac user can help figure this out. We do aim to be cross platform.

[zhelezov]

Alright, tossing suggestions in the blind, but here are few things to try:

• I don't know your setup but ~/.bash_profile is sourced if bash is a login shell. Try sourcing it manually in your shell session.
• Try a newer version of bash: brew install bash
• Try this patch:

diff --git a/shell-completion/hledger-completion.bash.stub b/shell-completion/hledger-completion.bash.stub
index 2c4862615..2c0c26aad 100644
--- a/shell-completion/hledger-completion.bash.stub
+++ b/shell-completion/hledger-completion.bash.stub
@@ -29,7 +29,11 @@
 
 _hledger_completion() {
     local cur prev words cword
-    _init_completion -n : || return 0
+    # _init_completion -n : || return 0
+    cword=$COMP_CWORD
+    words=("${COMP_WORDS[@]}")
+    prev=${words[cword-1]}
+    cur=${words[cword]}
 
     # Current treatment for special characters:
     # - exclude colon (:) from COMP_WORDBREAKS

I need to think this through so use the patch only for testing. I have local uncommitted work that relies on functions provided by bash-completion so I need to consider this more carefully before going that way.

Edit: _init_completion() was added with this commit on 20 Apr 2011: scop/bash-completion@32dbe76. It might be that bash-completion on mac is older than that.

[simonmichael]

https://itnext.io/programmable-completion-for-bash-on-macos-f81a0103080b is a good doc.
Apple is stuck on an old pre-GPL version of bash which isn't good for programmable completions.
A third party library, bash-completion, must be installed.

I think I'm still at the stage of getting bash completion libraries installed. I guessed this would get me running homebrew's bash 5.1, with bash-completion installed and _init_completion/__load_completion in scope, but not yet:

$ brew install bash bash-completion
$ /opt/homebrew/bin/bash
$ . /opt/homebrew/etc/profile.d/bash_completion.sh
$ type _init_completion
bash: type: _init_completion: not found
$ type __load_completion
bash: type: __load_completion: not found

I guess we don't have many mac users trying this. I'll update when I find something new, and will merge in any case.

[simonmichael]

Merged. Thanks @zhelezov for this epic update !

[simonmichael]

Success! With your patch.

[zhelezov]

Wait @simonmichael, did you just merge a PR that is not working for you?

BTW, a nice and comprehensive read the one you posted above. If I get it right, in order to make it work without patching on MacOS the user needs to upgrade bash and bash-completion with:

brew install bash
brew install bash-completion@2

Followed by some manual work to whitelist the new version of bash in /etc/shells and source bash_completion in the startup files. Anyway you can open an issue about this and we can address it either by updating the README for Mac users, or by patching it in a way that doesn't rely on bash-completion.

Or patch it in a way that degrades without fuss in case of missing/old/uninitialized bash-completion:

diff --git a/shell-completion/hledger-completion.bash.stub b/shell-completion/hledger-completion.bash.stub
index 2c4862615..b91d53c47 100644
--- a/shell-completion/hledger-completion.bash.stub
+++ b/shell-completion/hledger-completion.bash.stub
@@ -29,7 +29,12 @@
 
 _hledger_completion() {
     local cur prev words cword
-    _init_completion -n : || return 0
+    if ! _init_completion -n : 2>/dev/null; then
+        cword=$COMP_CWORD
+        words=("${COMP_WORDS[@]}")
+        prev=${words[cword-1]}
+        cur=${words[cword]}
+    fi
 
     # Current treatment for special characters:
     # - exclude colon (:) from COMP_WORDBREAKS

Can you give this a try, with and without bash-completion?

[simonmichael]

That's the stuff I tried. Unfortunately this is too much of a rabbit hole for me to spend more time on right now. I didn't want to release without this and PTA bash power users on mac seem to be a small niche. When someone figures it out though I bet our homebrew packagers will make it work. In fact we should ask them..

[simonmichael]

I see you added a new patch, I'll try it.

I forgot, these lines are still a problem on mac. Even when I run make from within homebrew's bash 5.1. Perhaps make still uses the default system bash.

ifneq ($(.SHELLSTATUS),0)
$(error Error running $(PARSE_COMMANDS))
endif

[simonmichael]

I jumped through the hoops again and all I got was this _init_completion error. Sorry, I'm sure this is awesome and I'd like to get on board with using shell completions but I don't have the patience to fiddle with it further. I reached out to one of our homebrew packagers, if they are interested we might get lucky.

[zhelezov]

Alright then, leaving it as it is for the moment, I will address any issues with bugfix PRs later. As to .SHELLSTATUS, it is a feature of GNU Make 4.2+ (2016). I guess your version is older than that. Can't think of an alternative out the top of my head. Its only purpose is to stop the build if parse-commands.sh fails because it runs outside any recipes and it will all result in a non-functional completion script. I will consider ways to refactor it.