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.

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

Target history #133

Open
petersutter opened this issue Mar 9, 2022 · 7 comments
Open

Target history #133

petersutter opened this issue Mar 9, 2022 · 7 comments
Assignees
Labels
component/gardenctl Gardener CLI kind/enhancement Enhancement, improvement, extension lifecycle/rotten Nobody worked on this for 12 months (final aging stage)

Comments

@petersutter
Copy link
Contributor

What would you like to be added:
It should be possible to see the target history, similar to the feature in gardener/gardenctl

Why is this needed:

@petersutter petersutter added component/gardenctl Gardener CLI kind/enhancement Enhancement, improvement, extension labels Mar 9, 2022
@petersutter petersutter mentioned this issue Mar 9, 2022
6 tasks
@tedteng
Copy link
Contributor

tedteng commented May 10, 2022

I am interested to do this task. the apprach probably similar from previous hisotry PR in gardenctl-v1 gardener-attic/gardenctl#512 use github.com/manifoldco/promptui for interaction.

one thing I like to change this time, proposal use native history file .zsh_history, .bash_history etc (guess it might different for powershell). main idea load those .xxx_history file filter gardenctl target and g target history command then used for history command. the advantage use of .xxx_history is central history place between different termianl sessions. in this way gardenctl with differnt sessionid should inherit the whole history recoard as well.

@petersutter
Copy link
Contributor Author

Hi @tedteng, I don't think it will be a good idea to use the shell history for this purpose. You wouldn't be able to reconstruct the target without huge interpretation effort, as you won't necessarily have the target always fully qualified (e.g. g target --garden my-garden --project my-project --shoot my-shoot) and the target could be constructed with separate commands (g target garden my-garden, g target seed my-seed). Apart from that you would need to implement parsers for each format.

We need to have the history in a structured format that we have under our control. What I'm not sure about is if the user wants to have the history only for the current session, or globally, or even both options.

Btw. github.com/manifoldco/promptui does not seem to be maintained actively

@dguendisch
Copy link
Member

I would also argue against the shell's history: it's a super unstructured interface, e.g. I use a very short alias for targeting: t, so in addition to what Peter said you'd need in addition to parse my aliases (but some people might use a shell function and not an alias) and you end up spending the rest of your lifetime in parsing edge cases :)
Instead (repeating what Peter said) gardenctl needs its own structured storage of a history.

Regarding session vs. global history: I think globally is more important but +1 on configurable :) (you could however bind the default to the shell's default, e.g. for zsh ohmyzsh/ohmyzsh#2537 (comment), but that's probably overenginering and not really required :) )

@tedteng
Copy link
Contributor

tedteng commented May 12, 2022

Thanks for the feedback. gather information will in the development later.

  • create history file save on local probability under ~/.garden and structured format control by us. in this way history feature enabled doesn't matter bash, zsh or PowerShell platform.
  • history structure data include GCTL_SESSION_ID and target data
  • current session, or globally, it could be No, I dont want share history. ohmyzsh/ohmyzsh#2537 (comment) , default set or changed by the flag, will see during the development.
  • github.com/manifoldco/promptui the search UI feature I really like it, welcome any option if there is another alternative.
    116015682-b9464100-a66c-11eb-8313-bb5cc3ff38b1

@petersutter
Copy link
Contributor Author

history structure data include GCTL_SESSION_ID [...]

My gut feeling is to have separate files (gardenctl home dir vs session dir)
-> If the user wants to have a session specific history, the history file is updated under the session directory
-> If the user wants to have a global history, the history file is updated under the gardenctl home directory

I wouldn't want to build a feature on a library that is not maintained anymore.

@tedteng
Copy link
Contributor

tedteng commented Jun 8, 2022

/assign
planning working on this ticket

Today, after rethinking about gardenctl home dir vs session dir. I'm planning to use global history unless there are some solutions we can handle session-id history, or separate tickets if the feature is needed and describe the use case.

from my previous experience, it seems doesn't make sense in reality when I use the v1 history feature for session dir/file. because we never have the same session id when a new terminal window opens, then the session history is always piece by piece saved in tons of different session folders, and never used by the target history command. unless terminal windows are not closed

probably interactive history records use/evaluate library by fuzzy
#135

@tedteng
Copy link
Contributor

tedteng commented Jun 22, 2022

History feature ready for review in PR #162

@gardener-robot gardener-robot added the lifecycle/stale Nobody worked on this for 6 months (will further age) label Jul 11, 2023
@gardener-robot gardener-robot added lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/gardenctl Gardener CLI kind/enhancement Enhancement, improvement, extension lifecycle/rotten Nobody worked on this for 12 months (final aging stage)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants