Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

updated shell completion #368

Merged
merged 1 commit into from
Aug 20, 2020
Merged

updated shell completion #368

merged 1 commit into from
Aug 20, 2020

Conversation

rsteube
Copy link
Collaborator

@rsteube rsteube commented Mar 2, 2020 •
edited
Loading

Had another go at what i originally extracted from the cobra zsh PR: https://github.com/rsteube/cobra-zsh-gen
This brings lab back to cobra (no fork) and makes the completion less cryptic as it's all go code.

Needs some good testing (looks ok so far).

fixes #333
fixes #367

@zaquestion
Copy link
Owner

zaquestion commented Mar 6, 2020 •
edited
Loading

go-git dependency is fine, in the long term I'd like to move everything over, I got many of the calls working a while back even https://github.com/zaquestion/lab/tree/go-git -- it's honestly considerably faster than exec'ing the binary.

@zaquestion
Copy link
Owner

zaquestion commented Mar 6, 2020 •
edited
Loading

@rsteube Tests are kinda fucky right now due to some changes on gitlab.com, I'm going to go ahead and merge this on faith since you've consistently delivered in the past. Also if you're interested in gaining contributor access so you can bring these through a little easier I'm super open to that.

zaquestion
zaquestion reviewed Mar 7, 2020
View reviewed changes
cmd/action/action.go Outdated Show resolved Hide resolved
@zaquestion
Copy link
Owner

Actually, ideally lets move the action package into internal and then we can bring this in.

@rsteube
Copy link
Collaborator Author

rsteube commented Mar 7, 2020

What does contributor access do? I am rather worried if means i can accidentally push to the wrong remote XD.

@rsteube rsteube mentioned this pull request Mar 15, 2020
Closed
@rsteube
Copy link
Collaborator Author

rsteube commented Mar 27, 2020

I managed to adapt this for fish and bash as well which i merged into a new project: https://github.com/rsteube/carapace
API is the same (just different package name) and what i tested looks pretty good so far so i'm updating this MR with it.

@rsteube rsteube changed the title updated zsh completion updated/added bash fish and zsh completion Mar 27, 2020
@zaquestion
Copy link
Owner

@rsteube Looks like the PR I just merged caused a conflict in this. Can you update and we'll get it brought in?

@zaquestion
Copy link
Owner

Contributor access would let you merge, I don't think anyone can push to master, but I would need to double check the controls

@rsteube
Copy link
Collaborator Author

rsteube commented Apr 20, 2020

Ok, thats fine then - i probably have to to fix some small quirks anyway when people start using it.

@rsteube rsteube changed the title updated/added bash fish and zsh completion WIP: updated/added bash fish and zsh completion Apr 20, 2020
@rsteube rsteube changed the title WIP: updated/added bash fish and zsh completion updated shell completion Apr 20, 2020
@zaquestion
Copy link
Owner

@rsteube I've added you as a collaborator. Not sure the state of the tests, so I (or someone) may need to fix those, ideally you'll be able to merge this on your own and bring in updates at your discretion

@zaquestion
Copy link
Owner

I can see the ci stuff is pretty borked at this point, and I introduced some conflicts bringing in a few other changes. Let's get the conflicts resolved and I'll bring this in and work to fix the pipelines so we can release

@rsteube
Copy link
Collaborator Author

rsteube commented Jul 28, 2020 •
edited
Loading

Nice - yeah at least at one point the url simply changed. I think it was sth like an added slash another "breadcrumb" in the repository url. (regarding the tests)

@rsteube
Copy link
Collaborator Author

rsteube commented Jul 29, 2020

@zaquestion seems the conflict was just an additonal import.
"Fixed" two tests, but seems the go-gitlab dependency needs an update: https://github.com/xanzy/go-gitlab/blob/master/project_snippets.go#L94
Code is renamed to Content (https://docs.gitlab.com/ee/api/project_snippets.html#create-new-snippet)

Apart from that the build itself is ok - shall i do a rebase first?

@rsteube
Copy link
Collaborator Author

rsteube commented Jul 29, 2020

Ok, seems updating that wasn't too hard. But now https://gitlab.com/lab-testing/test/-/merge_requests/1385 already exists, so the test fails.

@codecov
Copy link

codecov bot commented Jul 29, 2020 •
edited
Loading

Codecov Report

Merging #368 into master will increase coverage by 0.04%.
The diff coverage is 72.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
+ Coverage   63.56%   63.61%   +0.04%     
==========================================
  Files          49       50       +1     
  Lines        2709     2850     +141     
==========================================
+ Hits         1722     1813      +91     
- Misses        857      893      +36     
- Partials      130      144      +14
Impacted Files Coverage Δ
cmd/completion.go 40.00% <0.00%> (+20.00%) ⬆️
cmd/project_list.go 92.85% <ø> (-0.25%) ⬇️
cmd/root.go 71.42% <ø> (ø)
internal/action/action.go 11.11% <11.11%> (ø)
cmd/mr_checkout.go 82.35% <25.00%> (-7.97%) ⬇️
cmd/snippet_list.go 68.96% <57.14%> (+2.29%) ⬆️
internal/git/git.go 68.75% <68.00%> (-0.16%) ⬇️
cmd/mr_note.go 81.25% <80.00%> (+0.81%) ⬆️
cmd/mr_list.go 90.24% <82.60%> (-3.88%) ⬇️
cmd/issue_list.go 90.47% <83.33%> (-3.81%) ⬇️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f24bc8...7bebe48. Read the comment docs.

@rsteube
Copy link
Collaborator Author

rsteube commented Jul 29, 2020

@zaquestion well, the travis build exited with 0 but it still isn't reported.
This would explain it, but it's a rather old issue: https://travis-ci.community/t/known-issue-travis-ci-reports-expected-waiting-for-status-to-be-reported-on-the-github-status-api-but-the-status-never-arrives/1154

@rsteube
Copy link
Collaborator Author

rsteube commented Jul 30, 2020

moved ci fixes to separate PR: #386

@zaquestion
Copy link
Owner

@rsteube I think I got the travis stuff running again, I've gotta wait for a the checks to run on a PR before I can make them required, go ahead and try updating this branch to the latest they should run

@zaquestion
Copy link
Owner

Verfied the tests are passing with your fixes as well https://travis-ci.org/github/zaquestion/lab/builds/719466634

@rsteube
updated shell completion
7bebe48 
- uses github.com/rsteube/carapace
- thus no need for cobra fork anymore
- replaced completion_function.go with go callbacks
- provides shell completion for bash, elvish, fish, powershell and zsh
@rsteube rsteube merged commit df9161f into zaquestion:master Aug 20, 2020
@zaquestion zaquestion added this to the 0.18.0 milestone Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zaquestion zaquestion zaquestion left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.18.0
Development

Successfully merging this pull request may close these issues.

Add fish completions Zsh completion doesn't show "repo" alias for "project"
2 participants
@rsteube @zaquestion