github / gh-copilot

Ask for assistance right in your terminal.
https://docs.github.com/en/copilot/github-copilot-in-the-cli
618 stars 21 forks source link

[BUG]: Bash alias for `ghcs` doesn't maniuplate history correctly #65

Open bewuethr opened 3 months ago

bewuethr commented 3 months ago

What happened?

When looking at the shell history after using ghcs, the ghcs command itself seems to contain one extra field from history: the command number, or if HISTTIMEFORMAT is set, some timestamp.

My history related shell settings:

$ declare -p HISTTIMEFORMAT 
declare -- HISTTIMEFORMAT="%F %T "
$ shopt | grep hist
cmdhist         on
histappend      off
histreedit      on
histverify      on
lithist         on

Versions

$ gh version
gh version 2.46.0 (2024-03-20)
https://github.com/cli/cli/releases/tag/v2.46.0
$ gh copilot version
gh-copilot version 1.0.1 (2024-03-22)

Relevant terminal output

Running a command and looking at the three most recent commands in history:

$ ghcs 'print "Hi"'

Welcome to GitHub Copilot in the CLI!
version 1.0.1 (2024-03-22)

-->8--

Suggestion:                                                                                                                                                                                                          

  echo "Hi"                                                                                                                                                                                                          

? Select an option
> Execute command

? Are you sure you want to execute the suggested command?
> Yes

Hi
$ history 3
299027  2024-03-29 13:52:23 13:52:20 ghcs 'print "Hi"'
299028  2024-03-29 13:52:23 echo "Hi"
299029  2024-03-29 13:52:26 history 3

Notice how the third most recent command incorrectly includes the timestamp:

13:52:20 ghcs 'print "Hi"'
andyfeller commented 3 months ago

@bewuethr : thank you for opening this issue, this makes sense! 🙇

🤔 I see what you mean as I was able to reproduce this on Mac OS with Bash 5.2; in my version below the command's history ID is captured:

bash-5.2$ eval "$(gh copilot alias -- bash)"
bash-5.2$ ghcs print "hello world"

Welcome to GitHub Copilot in the CLI!
version 1.0.1 (2024-03-22)

I'm powered by AI, so surprises and mistakes are possible. Make sure to verify any generated code or suggestions, and share feedback so that we can learn and improve. For more information, see https://gh.io/gh-copilot-transparency

Suggestion:                                                                                                                                                                                                         

  echo "Hello, world!"                                                                                                                                                                                              

? Select an option
> Execute command

? Are you sure you want to execute the suggested command?
> Yes

Hello, world!
bash-5.2$ history 5
    8  gh copilot alias --help
    9  eval "$(gh copilot alias -- bash)"
   10  10 ghcs print "hello world"
   11  echo "Hello, world!"
   12  history 5
bash-5.2$ export HISTTIMEFORMAT="%F %T "
bash-5.2$ history 5
   10  2024-04-10 21:27:31 10 ghcs print "hello world"
   11  2024-04-10 21:27:31 echo "Hello, world!"
   12  2024-04-10 21:27:34 history 5
   13  2024-04-10 21:27:44 export HISTTIMEFORMAT="%F %T "
   14  2024-04-10 21:27:47 history 5
bash-5.2$ 10 ghcs print "hello world"

Reviewing the logic within the ghcs alias, I can see how the logic might be a bit brittle especially if you've customized how history is rendered with more fields:

        if [ -s "$TMPFILE" ]; then
            FIXED_CMD="$(cat $TMPFILE)"
            history -s $(history 1 | cut -d' ' -f4-); history -s "$FIXED_CMD"
            echo
            eval "$FIXED_CMD"
        fi

Since GA, we've discussed a different approach to executing commands through interactive shells, which hopefully works around th need to preserve the history this way.