Skip to content

Conversation

mbland
Copy link
Owner

@mbland mbland commented Jan 22, 2017

Closes #123. Tab completion will no longer change the working directory to _GO_ROOTDIR, the tradeoff being:

  • The compspec specifies -o nospace now instead of -o filenames.
  • This means that the ./go framework must add traling slashes to directory name completions, as well as add a space when returning a single completion (unless it ends with a /).
  • Tab-completed paths now won't trim the directory prefix from multiple matches when completing into more than one directory.

To this last point, per @jeffkole, having longer completion paths seems less surprising and annoying than having the working directory change. I'd originally thought it would mean completing absolute paths, but it turns out that wasn't necessary after all. In fact, the completed path on the command line stays the same length as before this change; it's just the values displayed when multiple matches exist that are longer.

The core of the @go.compgen solution from lib/complete is pretty straightforward, but since ./go complete will now add trailing slashes for directories, and trailing spaces when there is only one match, a lot of tests needed very minor updates.

Another bonus of this solution: Previously I named the test directory tests because tab-completing the test command would cause the previous implementation to add a slash if a test directory existed. Since we're not setting -o filenames, and are now filtering filename completions through @go.compgen, this should not be an issue for other projects. The "doesn't add trailing slashes when not called with -d or -f" test case from tests/complete/compgen.bats covers this.

While touching all these test files, I decided to apply a few other convenient updates, namely:

  • adding test_filter to setup
  • Replacing IFS=$'\n' with the updated assert_{success,failure} that accept multiple arguments to check for line-by-line equality

Closes #123. Tab completion will no longer change the working directory
to `_GO_ROOTDIR`, the tradeoff being:

- The compspec specifies `-o nospace` now instead of `-o filenames`.
- This means that the `./go` framework must add traling slashes to
  directory name completions, as well as add a space when returning a
  single completion (unless it ends with a `/`).
- Tab-completed paths now won't trim the directory prefix from multiple
  matches when completing into more than one directory.

To this last point, per @jeffkole, having longer completion paths seems
less surprising and annoying than having the working directory change.
I'd originally thought it would mean completing absolute paths, but it
turns out that wasn't necessary after all. In fact, the completed path
on the command line stays the same length as before this change; it's
just the values displayed when multiple matches exist that are longer.

The core of the `@go.compgen` solution from `lib/complete` is pretty
straightforward, but since `./go complete` will now add trailing slashes
for directories, and trailing spaces when there is only one match, a lot
of tests needed very minor updates.

Another bonus of this solution: Previously I named the test directory
`tests` because tab-completing the `test` command would cause the
previous implementation to add a slash if a `test` directory existed.
Since we're not setting `-o filenames`, and are now filtering filename
completions through `@go.compgen`, this should not be an issue for other
projects. The "doesn't add trailing slashes when not called with -d or
-f" test case from `tests/complete/compgen.bats` covers this.

While touching all these test files, I decided to apply a few other
convenient updates, namely:

- adding `test_filter` to `setup`
- Replacing `IFS=$'\n'` with the updated `assert_{success,failure}` that
  accept multiple arguments to check for line-by-line equality
@mbland mbland added the bug label Jan 22, 2017
@mbland mbland added this to the v1.4.0 milestone Jan 22, 2017
@mbland mbland self-assigned this Jan 22, 2017
@mbland mbland merged commit e953cd2 into master Jan 22, 2017
@mbland mbland deleted the tab-comp branch January 22, 2017 01:31
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 95.288% when pulling f52b6d8 on tab-comp into b103bd2 on master.

mbland added a commit that referenced this pull request Jan 22, 2017
Originally done during the course of implementing #123 and #124, when I
thought that `./go complete` needed to do some path manipulation. Still
seems a generally useful function.
mbland added a commit that referenced this pull request Jan 22, 2017
Originally done during the course of implementing #123 and #124, when I
thought that `./go complete` needed to do some path manipulation. Still
seems a generally useful function (even though I'm not using it yet).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tab completion changes directory
2 participants