target history feature #162

tedteng · 2022-06-15T05:54:38Z

What this PR does / why we need it:
target history feature and fuzzy targeting

Which issue(s) this PR fixes:
Fixes #133
Fixes #135

Special notes for your reviewer:
the Fuzzy targeting required fzf installed
the target history feature only records target history, which means target view, and target unset do not count in the implement
PowerShell is not supported yet.
use gh as alias name call gardenctl target history feature

Release note:

targeting history feature and Fuzzy targeting feature, use `gh` as alias name call `gardenctl target history` feature

tedteng · 2022-06-15T06:00:32Z

This PR is Ready for review
I don't have Win machine for Powershell alias code check. might need to create a separate ticket for this part

tedteng · 2022-06-29T07:43:35Z

This PR is ready for review, @gardener/gardenctl-v2-maintainers

petersutter · 2022-06-29T08:11:23Z

Hi Ted,

I don't have Win machine for Powershell alias code check

-> brew install pwsh on your Mac

I checked how the binary / history feature behaved and it reminds me of your very first implementation proposal in gardenctl (v1). You only store the command itself but loose the complete context. We already discussed this excessively in the past and you had improved it in the v1 implementation so that the context is not lost. That's why I'm a bit surprised why this implementation is again a step backwards, compared to the v1 implementation.

tedteng · 2022-06-29T14:05:46Z

The context mean cluster info is it? let me know if I get wrong understanding. if yes, that feature is provided by github.com/manifoldco/promptui , as we drop promptui package this feature is also dismissed.

Didn't realize the cluster info still needed to display it. as eval $(gardenctl kubectl-env zsh) in zshrc + PS1 it also displays after targeted to identify the environment.
but I just check the native feature provided by fzf. it seems also available. I will take a look it. it might not 100% look like before but I will have a try

petersutter · 2022-06-29T14:39:24Z

you do not only need the complete context for display purposes, but also to restore the target properly

tedteng · 2022-07-06T01:22:03Z

This PR is ready for review, https://github.com/orgs/gardener/teams/gardenctl-v2-maintainers

tedteng · 2022-07-18T23:52:54Z

Can we process this PR Thanks? @gardener/gardenctl-v2-maintainers

petersutter · 2022-07-19T09:50:18Z

Hi @tedteng, somehow it does not write the history in my case

When debugging it, it also did not run into PostRunE

gardenctl-v2/pkg/cmd/target/target.go

Lines 40 to 46 in 6be635b

    
           PostRunE: func(cmd *cobra.Command, args []string) error { 
        
           	s, err := HistoryParse(f, cmd) 
        
           	if err != nil { 
        
           		return err 
        
           	} 
        
           	return HistoryWrite(historyPath(f), s) 
        
           },

tedteng · 2022-07-25T00:58:29Z

Hi @petersutter, I'm just back from the vacation. Did the issue still occur? I was try to on my local and one more new Mac not meet this error again.

version

petersutter · 2022-08-02T14:35:52Z

@tedteng sorry for the late reply, yes the issue still occurs. @holgerkoser @grolu does it work for you?

use aliase name ghh, and use PersistentPostRunE fix alias and history write when target only

…it is an empty string

tedteng · 2023-01-16T03:01:08Z

PR is rebased feel free to process it thanks. @gardener/gardenctl-v2-maintainers

gardener-robot · 2023-04-27T12:07:20Z

@tedteng You need rebase this pull request with latest master branch. Please check.

tedteng · 2023-06-09T06:32:43Z

/close this PR. as it can simply create an alias after history to achieve the same effect. even in Win
eg

alias gg='eval $(history -n | grep -E "gardenctl target|g target" | grep -v history | uniq | fzf --tac  --no-sort --height 40% --layout reverse --info inline --border  --preview-window up,1,border-horizontal)'

tedteng requested a review from a team as a code owner June 15, 2022 05:54

gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Jun 15, 2022

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 15, 2022

gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jun 15, 2022

tedteng force-pushed the history branch from 030594b to 93246a4 Compare June 17, 2022 01:53

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 17, 2022

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 17, 2022

This was referenced Jun 22, 2022

Target history #133

Open

Fuzzy targeting #135

Open

tedteng marked this pull request as draft June 30, 2022 07:48

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 5, 2022

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 5, 2022

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 5, 2022

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 5, 2022

tedteng force-pushed the history branch from 6341d3c to 6be635b Compare July 6, 2022 01:17

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 6, 2022

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jul 6, 2022

tedteng marked this pull request as ready for review July 6, 2022 01:21

tedteng force-pushed the history branch from b952387 to c01507a Compare November 9, 2022 13:41

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 9, 2022

tedteng force-pushed the history branch from c01507a to 0702651 Compare November 9, 2022 13:47

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 9, 2022

tedteng added 11 commits January 16, 2023 10:01

history feature

8978536

fix full target history

4ba5e49

add display and add powershell alias

1e1d93c

use aliase name ghh, and use PersistentPostRunE fix alias and history write when target only

add code to adapt target issue

f7740f4

PR Feedback

51485ff

introduce history-env

ba3980f

PR Feedback

e097e9a

update unit test

f80466b

fix history powerhsell and cannot bind argument to parameter because …

7611078

…it is an empty string

PR Feedback Test

03691e3

options and history tmpl

d688754

tedteng force-pushed the history branch from 0702651 to d688754 Compare January 16, 2023 02:02

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 16, 2023

rebase

2212f11

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 16, 2023

gardener-robot added the needs/rebase Needs git rebase label Apr 27, 2023

tedteng mentioned this pull request May 19, 2023

introduce plug-in feature in gardenctlv2 #299

Closed

gardener-robot closed this Jun 9, 2023

gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Jun 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

target history feature #162

target history feature #162

tedteng commented Jun 15, 2022 •

edited

Loading

tedteng commented Jun 15, 2022 •

edited

Loading

tedteng commented Jun 29, 2022

petersutter commented Jun 29, 2022

tedteng commented Jun 29, 2022 •

edited

Loading

petersutter commented Jun 29, 2022

tedteng commented Jul 6, 2022

tedteng commented Jul 18, 2022

petersutter commented Jul 19, 2022

tedteng commented Jul 25, 2022

petersutter commented Aug 2, 2022

tedteng commented Jan 16, 2023

gardener-robot commented Apr 27, 2023

tedteng commented Jun 9, 2023 •

edited

Loading

target history feature #162

target history feature #162

Conversation

tedteng commented Jun 15, 2022 • edited Loading

tedteng commented Jun 15, 2022 • edited Loading

tedteng commented Jun 29, 2022

petersutter commented Jun 29, 2022

tedteng commented Jun 29, 2022 • edited Loading

petersutter commented Jun 29, 2022

tedteng commented Jul 6, 2022

tedteng commented Jul 18, 2022

petersutter commented Jul 19, 2022

tedteng commented Jul 25, 2022

petersutter commented Aug 2, 2022

tedteng commented Jan 16, 2023

gardener-robot commented Apr 27, 2023

tedteng commented Jun 9, 2023 • edited Loading

tedteng commented Jun 15, 2022 •

edited

Loading

tedteng commented Jun 15, 2022 •

edited

Loading

tedteng commented Jun 29, 2022 •

edited

Loading

tedteng commented Jun 9, 2023 •

edited

Loading