Run the bash_completion project

[andychu]

See also #175 (Run git-completion.bash)

Things that don't work:

• printf -v var %q foo -- should be changed to var=${foo@Q}
• ${!x} where x=a[i]. PR by Greg Price
• ${!x#pattern} -- var ref then prefix/suffix op. Reported by Greg Price.
• Make local "$2" work and revert the eval patches (I think). This is related to static vs. dynamic parsing, mentioned in cleanup: rationalize ${a[x]} and $(( a[x] )) #207.
  • This change is somewhat invasive because parsing assignments is somewhat subtle. There is some inconsistency between export and local/readonly/declare that I would like to resolve. Right now export is only dynamically parsed, but local/readonly/declare are statically parsed.

TODO: What else?

[andychu]

7521118 fixes some issues with ${!x}

[gnprice]

I just made a quick skim through the patches in
scop/bash-completion@master...oilshell:dev/osh-compat
to find things that that patched out that might be reasonable to implement.

• printf -v array[i] -- printf -v "$2[i]" appears in one place in the tip commit. Other printf -v "$foo" could also turn out to be array refs.
• $array meaning the same as ${array[0]} -- This accounts for two of the patches, I think. Seems like basically any script relying on this behavior would be improved by a patch like these; but also shouldn't be too hard to implement, and my suspicion is a lot of scripts do this.
• local "$2" -- I don't totally understand what's happening here. There are a couple of iterations on this patch; seems like the first workaround caused some other issue?
• BASH_VERSINFO -- OK, patching this is definitely the only reasonable solution 🙂

[andychu]

Part of the goal of OSH is also to be saner, so if there is a trivial patch like $array -> ${array[0]} then I'm inclined to keep it. Possibly later there could be a flag to enable "not sane" behavior if we run into some scripts that are really hard to change.

I think it's probably time to contact the maintainers of bash-completion and see how amenable they are to patches that will work in both bash and OSH. This will obviously be a lot more convincing if we can run the whole project :)

Whether we have to maintain a fork will probably influence some decisions. But since all the patches are one-liners, I think it will be easy to maintain a fork if we have to (hopefully not for too long).

• Yeah the eval 'local "$2"' patches should be reverted. I will edit the checklist. That is related to what's mentioned in cleanup: rationalize ${a[x]} and $(( a[x] )) #207 about static vs. dynamic parsing of local.

• Are you saying that printf -v also has to have the same logic to parse array refs as ${!x} and unset a[i] ? If so that is good to know.

[gnprice]

• Are you saying that printf -v also has to have the same logic to parse array refs as ${!x} and unset a[i] ? If so that is good to know.

Yup, looks like it. Hopefully exactly the same logic, so we can largely share the implementation.

Part of the goal of OSH is also to be saner, so if there is a trivial patch like $array -> ${array[0]} then I'm inclined to keep it. Possibly later there could be a flag to enable "not sane" behavior if we run into some scripts that are really hard to change.

Reasonable. It's certainly cheap to stick to that position here, since this script already needs some other patches -- BASH_VERSINFO if nothing else -- and this is the one that (unless we're missing something subtle about it) should be the easiest for the maintainers to take.

[andychu]

Update: Much more of this runs, and everything I tested in git-completion.bash also runs.

Big takeaway: bash-completion has a bash parser in bash !!! It tries to parse redirects, variable names, etc. I replaced 3 of these nasty functions, more than 300 lines of code, with a simple compadjust builtin and the actual OSH parser:

$ wc -l bash_completion osh_completion 
  2115 bash_completion
  1786 osh_completion

In other words, OSH has a significantly different architecture than bash, because it actually uses its own parser to complete the language. There are really two separate problems:

• Completing the bash/OSH language itself
• Completing command line options

Bash conflates these two things, which makes it essentially impossible to do a correct job, but OSH doesn't.

There are still bugs:

• The return 124 protocol results in needing a double tab for some reason
• printf -v. I disabled this since some other tests started failing. I'm hoping to get rid of this construct, since bash-completion is the only project I've run so far that uses it.
• ssh plugin aborts because OSH is stricter with arrays vs. strings. Should be simple to fix.
• the _filedir function fails, may need to be replaced

Big commits:

f6de2e9

e485ac4

---

@gnprice Thanks for helping out with this! When I checked the status of the code a couple weeks ago, I realized the indirect array references you implemented pushed it over the edge to really running. It seems like that was one of the last big blockers, and most other things are polish.

I gave you a shout-out here:

http://www.oilshell.org/blog/2018/12/16.html#toc_4

Although this "breakthrough" with compadjust leaves me hopeful that I can avoid implementing some of the stuff I mentioned there. The goal is still to "write a manual with a straight face" and getting rid of the bash parser in bash actually does make the required language simpler!

[Crestwave]

• printf -v. I disabled this since some other tests started failing. I'm hoping to get rid of this construct, since bash-completion is the only project I've run so far that uses it.

I've seen it used a decent amount of times, e.g., https://github.com/dylanaraps/neofetch, https://github.com/dylanaraps/birch, my own https://github.com/Crestwave/brainbash, etc.

[andychu]

Yeah I suspect in the long term we may need printf -v. It looks like neofetch is pretty popular!

We have a partial implementation of printf, but doing the whole thing is a "schlep" with a lot of corner cases. I would definitely accept a patch for it :)

Another approach to consider is to simply copy some C code that implements it. Every shell has a pretty compatible implementation, and it's self-contained. The -v means crossing the Python/C boundary to write to state.Mem(), but that's not a big hurdle.

[andychu]

This is done (and printf -v was just done).

Some patches to the bash-completion project are necessary, but that's intentional... I removed the bash parser in bash :-/