-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Make gh cs
aware it is inside a codespace
#9040
Comments
The docs mention a few environment variables present in any codespace, the interesting one being:
Using this environment variable as a default for some commands could be a simple and easy change. E.g. diff --git a/pkg/cmd/codespace/codespace_selector.go b/pkg/cmd/codespace/codespace_selector.go
index a51e42b6..00491412 100644
--- a/pkg/cmd/codespace/codespace_selector.go
+++ b/pkg/cmd/codespace/codespace_selector.go
@@ -24,7 +25,7 @@ var errNoFilteredCodespaces = errors.New("you have no codespaces meeting the fil
func AddCodespaceSelector(cmd *cobra.Command, api apiClient) *CodespaceSelector {
cs := &CodespaceSelector{api: api}
- cmd.PersistentFlags().StringVarP(&cs.codespaceName, "codespace", "c", "", "Name of the codespace")
+ cmd.PersistentFlags().StringVarP(&cs.codespaceName, "codespace", "c", os.Getenv("CODESPACE_NAME"), "Name of the codespace")
cmd.PersistentFlags().StringVarP(&cs.repoName, "repo", "R", "", "Filter codespace selection by repository name (user/repo)")
cmd.PersistentFlags().StringVar(&cs.repoOwner, "repo-owner", "", "Filter codespace selection by repository owner (username or org)") I think that if This would change the behavior only of sessions inside a codespace by default (i.e. unless you add this variable yourself). And in this case, I think it's fair to think that And if someone want to access a different codespace from the current one, then Pretty much what |
@alondahari : Thanks for sharing this idea for improving I defer to @dmgardiner25 and @cmbrose if there some other consideration. Beyond that, I would suggest using an approach similar to |
Thanks for the suggestion! I think this makes a lot of sense to add. I would lean in the same direction as @andyfeller that we shouldn't have the value appear in the I'd just want to make sure that we're careful with the Maybe we could just handle that edge case in the |
Honestly, that sounds like a good confirmation prompt to have even if you select the codespace manually from a list. Ah, nevermind it's already there 🎉 |
Just to call it out, that will only trigger if you have uncommitted changes in the codespace. So if you've already pushed your changes (or simply haven't made any edits to the repo content) then just running |
Aight, then I would definitely include some guardrails first, with respect for the If I understand correctly, this is the current flow: flowchart TD
command[gh cs delete] --> select[> select codespace]
command -- flag: codespace=abc --> state
select --> state[codespace's state]
state -- dirty --> confirmation[> Do you want to delete?]
state -- flag: force=true --> delete
state -- clean --> delete
confirmation -- yes --> delete
confirmation -- no --> keep
I think the change should be: flowchart TD
command[gh cs delete] --> select[> select codespace]
command -- flag: codespace=abc --> state
command -- ENV: CODESPACE=abc --> state
select --> state[codespace's state]
state --> confirmation[> Do you want to delete?]
state -- flag: force=true --> delete
confirmation -- yes --> delete
confirmation -- no --> keep
|
I think that's the right direction, but this is what I was trying to describe: flowchart TD
command[gh cs delete] --> select[> select codespace]
command -- flag: codespace=abc --> force
command -- ENV: CODESPACE=abc --> force
select --> force[flag: force]
force -- true --> delete
force -- false --> name[ENV: CODESPACE == selected codespace]
name -- false --> dirty[state == dirty]
name -- true --> confirmation[> Do you want to delete?]
dirty -- false --> delete
dirty -- true --> confirmation
confirmation -- yes --> delete
confirmation -- no --> keep
|
Thank you for putting thought into this, very exciting! I would add my 2 cents and argue that if I run Maybe if the cs is dirty, we can update the confirmation message to make it clear that the cs that you are deleting is indeed the one you are in, and your session will end? |
Describe the feature or problem you’d like to solve
Trying to get some comments on the idea of making the
gh cs
set the--codespace
flag automatically if it's executed from inside a codespace. For example, if I rungh cs ports
in a codespace, I probably want to get the ports exposed by the codespace I'm in. However, this is probably not the intention when executinggh cs code
.For this proposal to go forward, we probably need to put some work into figuring out for which sub commands this default would work for. Since it would probably be a subset of commands, we should have clear reasoning behind it so we can document it and have a behaviour that the user can expect.
Proposed solution
On select sub commands of
gh cs
, if run from inside a codespace, the--codespace
flag should default to the current codespace. The user should be able to override it by passing the--codespace
flag explicitly.As an alternative, we could add a
--current
flag as sugar instead, to make the behaviour opt-in.The text was updated successfully, but these errors were encountered: