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

[FEAT]: Command to run code #2

Closed
asportnoy opened this issue Nov 8, 2023 · 17 comments
Closed

[FEAT]: Command to run code #2

asportnoy opened this issue Nov 8, 2023 · 17 comments
Assignees
Labels
enhancement New feature or request

Comments

@asportnoy
Copy link

Describe the need

One feature that the GitHub next beta had which this is missing is being able to automatically run the command. This version only has a "copy" option, and to make it worse, you have to press "exit" after copying before you can actually do anothing with the command. This a much worse user experience.

Few things I think could be improved here:

  • Automatically exit after copying the command
  • Add an option to run the command as a primary action instead of copy.
  • Add an option to output the command to a file like with the beta, which makes it possible to build this better into our own workflows

Version

v0.5.2-beta

Relevant terminal output

N/A

@asportnoy asportnoy added enhancement New feature or request needs-triage needs to be reviewed labels Nov 8, 2023
@andyfeller
Copy link
Contributor

@asportnoy : Thank you for creating this issue! 🙇

This was a difficult question the team asked itself when rewriting the GitHub Next technical preview: "How important was being able to execute the code? What are the ramifications if it was destructive or harmful?".

At the time, we erred on the side of focusing on the rewrite while hearing back from our users. That said, we have heard this feedback also from Hubbers in the weeks leading up to Universe, so I believe I can say the team is open to the idea.

@andyfeller andyfeller removed the needs-triage needs to be reviewed label Nov 8, 2023
@asportnoy
Copy link
Author

The GH Next preview had a confirmation prompt after choosing "execute" to avoid people accidentally running harmful commands, which could definitely be used here.

Another potential option which I used for my custom Fish Shell wrapper is to have it set the prompt (using the builtin commandline command) but not actually execute it, which also has the advantage of allowing me to tweak the command myself before running it. However, I'm not sure if other shells have a way to set the prompt, so that may be an issue.

Regardless, making it exit after copying is an easy step which I think should be done regardless. Also adding the optional flag to output the command to a file lets people build custom wrappers like I made for Fish which would allow people to make it execute automatically if they prefer, or to do other cool things with it as well.

@hamadzidani
Copy link

hamadzidani commented Nov 8, 2023

The GH Next preview had a confirmation prompt after choosing "execute" to avoid people accidentally running harmful commands, which could definitely be used here.

Another potential option which I used for my custom Fish Shell wrapper is to have it set the prompt (using the builtin commandline command) but not actually execute it, which also has the advantage of allowing me to tweak the command myself before running it. However, I'm not sure if other shells have a way to set the prompt, so that may be an issue.

Regardless, making it exit after copying is an easy step which I think should be done regardless. Also adding the optional flag to output the command to a file lets people build custom wrappers like I made for Fish which would allow people to make it execute automatically if they prefer, or to do other cool things with it as well.

I use Fish as well, could you tell how you set that wrapper to set the prompt?

@0xdevalias
Copy link

0xdevalias commented Nov 8, 2023

could you tell how you set that wrapper to set the prompt?

@hamadzidani A little ChatGPT and a little Google suggests:

  • https://fishshell.com/docs/current/cmds/commandline.html
    • commandline can be used to set or get the current contents of the command line buffer.

      With no parameters, commandline returns the current value of the command line.

      With CMD specified, the command line buffer is erased and replaced with the contents of CMD.

As for other shells, this is what ChatGPT had to say (I gave this a quick test, and it seemed to work):

In other shells like bash, there isn't a built-in function exactly like commandline in fish. However, you can use a similar trick by binding a keyboard shortcut to an echo command that prints out the command you want to stage. Here's an example of how you might do this in bash using bind:

bind '"\C-p": "echo This is the command"'

This bash command binds Ctrl+p (written as \C-p) to output echo This is the command to the command line. The command will be on your command line, but it won't execute until you press Enter.

Please note that the exact syntax and capabilities can vary between different shells and their respective versions.

And for zsh (I did test this, and it seems to work):

In zsh, you can use the print -z command to achieve a similar effect to fish shell's commandline. The print -z command puts a command into the command buffer (like typing it out), but doesn't execute it. Here's a simple example:

print -z "echo 'This is the command'"

When you run this command in zsh, it will populate the command line with echo 'This is the command', but it won't execute until you press Enter.

If you want to bind this to a keyboard shortcut in zsh, you can use the bindkey command. Here’s how you can bind this to a keyboard shortcut, for example, Ctrl+x, Ctrl+e:

bindkey '^X^E' my-prep-command

And you'd have a function my-prep-command that looks like this:

function my-prep-command {
  BUFFER="echo 'This is the command'"
  zle end-of-line
}
zle -N my-prep-command

This sets up a zsh line editor (zle) widget called my-prep-command that, when invoked, sets the current command line (BUFFER) to echo 'This is the command' and then moves the cursor to the end of the line with zle end-of-line. The bindkey line binds this widget to the specified key sequence.

Further reading on print -z:

Further reading about the zsh line editor:

@spenserblack
Copy link

and to make it worse, you have to press "exit" after copying before you can actually do anothing with the command.

Piggybacking on this to suggest that maybe, once the user has selected Copy command to clipboard, the default selected option could be Exit instead of Copy command to clipboard once again. I'm assuming that users will want to exit and run the command after copying a majority of the time. At least, it seemed strange to me that after selecting Copy command to clipboard that choice was still there and selected.

@andyfeller
Copy link
Contributor

Firstly, I just want to say I love the collaboration! 😻

As I mentioned in #5 (comment), I think the proper way to support this is by implementing something similar to what was done in the GitHub Next technical preview. That said, this alias wrapper solution was very Unix/Linux friendly but not so much PowerShell and possibly others.

@yongkangchen
Copy link

Greetings! To facilitate command execution, I've established cps to wrap the Copilot CLI command.
You can find it at https://github.com/yongkangchen/cps/tree/main.

@Wladefant
Copy link

Wladefant commented Nov 25, 2023

@yongkangchen nice. Yes honestly @andyfeller too much prompts, the thing about cli is that you can do it fast, or otherwise you would just go to chatgpt or copilot. I even found that the confirmation was too much. We need something very simple.

For example:
1.?? "show files"
2. thinking...
answer: ls
(then like before directly the explanation (very important to not run into problems))
run this command [y/n/Revise]
3. enter is yes n is no, r is revise
4. that is it

No more no less or I would just go to chatgpt

@vuon9
Copy link

vuon9 commented Nov 27, 2023

It's a bit annoying when the behavior is different to github-copilot-cli npmjs package behavior. Copy should have an option to immediately quit the interactive UI after copied and also will be the best to have another option to output command to anywhere else (cli, file).

@andyfeller
Copy link
Contributor

It's a bit annoying when the behavior is different to github-copilot-cli npmjs package behavior. Copy should have an option to immediately quit the interactive UI after copied and also will be the best to have another option to output command to anywhere else (cli, file).

@vuon9 : hear you, loud and clear 👂

@OriNachum
Copy link

First, I really love this and the effort and work behind it!

About the feature. I actually expected it to provide the command right away, instead of asking me questions back.

Questions should be once, reconfiguration allowed (like Custom Instructions)

Then it works the same with same principles for all following calls.

Then I expect 1 call = 1 command.
No auto execute, of course, but 1 click away.
If I want to correct, I tap "up" and make changes.

Experience should be smooth, like part of the flow of working on CLI

@asportnoy
Copy link
Author

Bit disappointed that the GitHub Next version of the Copilot CLI is going away today as I feel like this version is a downgrade in its current state and much harder to use. Do you have an ETA for when some of the features discussed here will be implemented? Are you able to extend the sunset for the beta until that happens?

@0xdevalias
Copy link

0xdevalias commented Dec 16, 2023

Bit disappointed that the GitHub Next version of the Copilot CLI is going away today

@asportnoy This was in the email sent this morning, that suggests otherwise:

image

@asportnoy
Copy link
Author

Didn't get that for some reason, thank you.

@angelhof
Copy link

@asportnoy : Thank you for creating this issue! 🙇

This was a difficult question the team asked itself when rewriting the GitHub Next technical preview: "How important was being able to execute the code? What are the ramifications if it was destructive or harmful?".

At the time, we erred on the side of focusing on the rewrite while hearing back from our users. That said, we have heard this feedback also from Hubbers in the weeks leading up to Universe, so I believe I can say the team is open to the idea.

It makes a lot of sense to err on the conservative side when running code without proper inspection. One idea to consider could be to have an option of running the command in a sandbox/lightweight container to not accidentally do something harmful to the user's system. One way to do it would be with a tool like try (disclaimer: I am one of the developers), which runs a command in a lightweight container that is created using Linux namespaces and overlayfs (similarly to docker containers). It has minimal dependencies (it is just a shell script) and can be run very easily. @andyfeller if this is something that people think is useful we would be happy to put some work in it!

@noahkiss
Copy link

One UX point that hasn't been discussed, and which doesn't change any current functionality:

Exit should be renamed to Exit (Quit) or similar.

? Select an option  [Use arrows to move, type to filter]
> Copy command to clipboard
  Explain command
  Revise command
  Rate response
  Exit

Right now, the user either needs to move their hand to the arrow keys to select Exit or type to filter it, which requires typing e, x, i before Exit is selected.

Renaming to Exit (Quit) or otherwise filtering on Quit would allow for one key [Q] to select the Exit option rather than three.

While this doesn't solve the problem of immediate pasting, it at least cuts down on keystrokes between command and exiting.

(Another option would be a Copy and Exit (Quit) command which I believe would go even further in satisfying users)

@andyfeller
Copy link
Contributor

@asportnoy : I'm genuinely happy to say your feedback has been heard as of v1.0.0 with GitHub Copilot in the CLI GA that support for executing commands, alias helpers, and even shortcircuiting copy clipboard functionality have all been released!

I can't wait to hear back from you and our other users if this moves the needle toward being the Copilot you need. ❤️

For more information, see https://github.blog/changelog/2024-03-21-github-copilot-general-availability-in-the-cli/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests