Added script to get collection roots from packages for zsh auto-complete #499

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

Contributor

This fixes the zsh auto-complete feature to work with both the standard collection paths as well as the collection roots from the various links files. It doesn't work with individually installed collections, though, which would require more changes to the .zsh script.

Owner

This should really use list-collects.rkt instead of creating another copy, possibly making it depend on a shell variable to do things that are specific to each shell (but IIRC, they should both use it in the same way).

(BTW, you left the error message when you copy-pasted it...)

Owner
samth commented Mar 31, 2014

@schuster ping

Contributor

@samth pong. I haven't touched this since the original PR. Did you have a specific queston?

Owner
samth commented Mar 31, 2014

Well, @elibarzilay had some specific suggestions in his comment.

Contributor
schuster commented Apr 7, 2014

I've updated the code. It all uses a common list-collects.rkt, but zsh and bash have to use it in two different ways because of the expected output of the shell scripts. Roughly speaking, bash expects a list of collection names, while zsh expects a list of directories that contain collection directories (the collection roots and parent directories for one-off collections). So there's a separate Racket program for each shell.

Ideally each shell would do the same thing, but I don't have the bash/zsh-fu to know if that's even possible, much less implement it in a reasonable amount of time.

Owner

It's not really related to knowing bash/zsh. The thing is that once you have two files, there's a place to write different code in each, and once there's such a place, people will use it. It's easy to just put the two bits in the same file. You can even add an environment variable to identify each shell, and then there's even no need for flags or whatever -- just dispatch based on the variable. Combining these two things, results in the following: https://gist.github.com/elibarzilay/10014970
(untested shell code, but there's little that chaged)

Owner
samth commented Apr 7, 2014

Even easier would be to put the two functions in one file, and have two submodules, one of which does one thing and one the other.

Owner

10 minutes ago, Sam Tobin-Hochstadt wrote:

Even easier would be to put the two functions in one file, and have
two submodules, one of which does one thing and one the other.

That's probably easier in terms of refactoring, but there's really
very little difference between the two bits. (And since it's still
just completion related work, the chances of the two sides staying
very similar are high, and the profits are high too since there's not
that many shell experts around.)

      ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                http://barzilay.org/                   Maze is Life!
Contributor
schuster commented Apr 9, 2014

Just pushed a version that uses submodules. This seems more direct than shell variables: the shell scripts can call to their entry points directly instead of setting a flag and relying on the Racket program to branch correctly based on that.

Owner
  1. This is a shell completion thing -- relying on environment
    variables is as justified as you could ever get to: the shell code
    that uses it is itself relying heavily on such things.
  2. You ignored the rest of my changes, and left in the obscure use of
    a hash table, which is (IIUC) a very obfuscated way to do the same
    thing that `remove-duplicates' gives you.
Contributor
schuster commented Apr 9, 2014

Sorry, I didn't notice your other changes. I might be able to take a look later this week/weekend. Or if your code fixes the original bug, I don't mind that being pushed instead. This was just intended to be a small bug fix, so the less work that has to happen, the better.

Owner

IMO it's important to keep things in a shape that makes sharing code
on future improvements easy, and doesn't encourage splittage (and
different modules still make it easy in that sense).

Contributor
stamourv commented Apr 9, 2014

Eli, does the code you posted solve the issues @schuster's pull request solves? If so, should we use it instead? (after testing, of course)

Owner

It should, since it's basically doing the same thing.

Contributor
schuster commented Apr 9, 2014

Yeah, I just had a chance to do a quick diff: I thought it was a much larger change (gist didn't show the diff, unfortunately), but I see now it's just related to the shell variable stuff, remove-duplicates, and some variable renaming. I'll go ahead and make those changes and push.

@schuster @schuster schuster Fix zsh auto-completion
The bash and zsh scripts expect different data from Racket: zsh expects
a list of collect roots and other parent directories of collections,
while bash expects a list of collection names
b1362bc
Contributor
schuster commented Apr 9, 2014

Pushed. Had to change the shell scripts to require the main submodule explicitly, since (require shell-completion/list-collects) won't require it automatically. If we'd be better off just getting rid of the submodule and putting that code in the body of the file, let me know and I can change it.

Contributor
stamourv commented Apr 9, 2014

Merged, thanks!

@schuster schuster closed this Apr 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment