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

Make gh cs aware it is inside a codespace #9040

Open
alondahari opened this issue May 2, 2024 · 8 comments
Open

Make gh cs aware it is inside a codespace #9040

alondahari opened this issue May 2, 2024 · 8 comments
Labels
enhancement a request to improve CLI gh-codespace relating to the gh codespace command

Comments

@alondahari
Copy link

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 run gh 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 executing gh 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.

@alondahari alondahari added the enhancement a request to improve CLI label May 2, 2024
@cliAutomation cliAutomation added the needs-triage needs to be reviewed label May 2, 2024
@williammartin williammartin added the gh-codespace relating to the gh codespace command label May 2, 2024
@nobe4
Copy link
Contributor

nobe4 commented May 2, 2024

The docs mention a few environment variables present in any codespace, the interesting one being:

CODESPACE_NAME	The name of the codespace For example, octocat-literate-space-parakeet-mld5

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 CODESPACE_NAME is present in the env, then it should be used as a default value for --codespace.

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 gh cs means "for the current codespace".

And if someone want to access a different codespace from the current one, then --codespace is available.

Pretty much what gh repo ... does. By default it's trying to find the github remote and infer the repo from it, but if I want to change a different one, I need to make it explicit with --repo.

@andyfeller
Copy link
Contributor

@alondahari : Thanks for sharing this idea for improving gh codespace! Really appreciate @nobe4 already jumping on ideas to make this happen ❤

I defer to @dmgardiner25 and @cmbrose if there some other consideration.

Beyond that, I would suggest using an approach similar to GH_HOST lookup for an override before going into different logic versus using a dynamic environment variable for a Cobra command flag value. Main reason being it changes --help output within a Codespace versus local workstation. Take with a grain of 🧂

@andyfeller andyfeller removed the needs-triage needs to be reviewed label May 7, 2024
@cmbrose
Copy link
Contributor

cmbrose commented May 7, 2024

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 --help text (but it would be good to call out that message that we'll use the default in a codespace).

I'd just want to make sure that we're careful with the delete command using the implicit codespace name. If a user runs that in a codespace expecting to get the prompt list, they would actually delete the codespace they are running the command in!

Maybe we could just handle that edge case in the delete command to prompt "are you SURE you want to delete the codespace you are currently in??"

@nobe4
Copy link
Contributor

nobe4 commented May 8, 2024

Maybe we could just handle that edge case in the delete command to prompt "are you SURE you want to delete the codespace you are currently in??"

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 🎉

@cmbrose
Copy link
Contributor

cmbrose commented May 8, 2024

Maybe we could just handle that edge case in the delete command to prompt "are you SURE you want to delete the codespace you are currently in??"

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 gh cs delete from within a codespace with this functionality enabled would result in the codespace suddenly getting deleted out from under you.

@nobe4
Copy link
Contributor

nobe4 commented May 9, 2024

with this functionality enabled would result in the codespace suddenly getting deleted out from under you.

Aight, then I would definitely include some guardrails first, with respect for the --force flag. I expect things to happen without confirmation prompt whenever I use --force.

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

@cmbrose
Copy link
Contributor

cmbrose commented May 9, 2024

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

@alondahari
Copy link
Author

Thank you for putting thought into this, very exciting!

I would add my 2 cents and argue that if I run gh cs delete from within a clean codespace and it's deleted from under me, I wouldn't be too upset. It might actually be a nice easy way to end my current session. If nothing is lost anyway, the price of a mistake here is minimal.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a request to improve CLI gh-codespace relating to the gh codespace command
Projects
None yet
Development

No branches or pull requests

6 participants