benthayer / git-gud

Wanna git gud? Then get git-gud, and git gud at git!
MIT License
401 stars 42 forks source link

"git gud explain": Do not accept user input other than ENTER #332

Open benthayer opened 3 years ago

benthayer commented 3 years ago

Go to util/level_builder.py in the function BasicLevel.explain() to fix this.

Basically git gud explain waits for the user to hit enter, but they can also type anything they want because we use input and not something more advanced. This could mess up input or simply make it more confusing for a user, with them possibly thinking they're typing into a command line.

vmdhhh commented 3 years ago

@benthayer We could also start with giving a small text saying something like "Press Enter" and also take care of the user input with few more steps What do you think ?

benthayer commented 3 years ago

Hello @vmdhhh , thanks for commenting!

We do already have something like what you said to tell the user to be pressing enter, so I think you have the right idea. You can see here in the third level of the introduction (intro init), we have the user run git gud explain and then they see this:

Before learning about Git repos, hit enter
>>> (1/9)
Explanations are broken up into multiple chunks to make it easier to read.
>>> (2/9)
Hitting enter will get you to the next part of the explanation
>>> (3/9)
...

Was that in line with what you were thinking?

Beyond that, it's just about making sure that if someone types a character other than enter, it doesn't get displayed

benthayer commented 3 years ago

@vmdhhh Feel free to handle this one too: https://github.com/benthayer/git-gud/issues/333

vmdhhh commented 3 years ago

@benthayer Did you already take care of this? Because now I see that the typed text is getting replaced.

benthayer commented 3 years ago

Yeah, I've worked on this, but users shouldn't be able to type at all because it's confusing for the people who aren't familiar with the command line and think the line they're seeing is where they type out commands. So, when they hit enter, the command disappears and they don't know what happened.

The current behavior exists so that when a user hits enter, that enter is removed and we start print()ing on that line. We need the extra newline because after the final explanation block, the command line is displayed and offsets our output by one line

vmdhhh commented 3 years ago

I am able to implement this using the keyboard package with a minor problem. Problem: If the user types something instead of enter while the explain is running, it's getting displayed on the terminal after the program terminates. I have attached an image for reference. Sorry for the bad image edit :( image @benthayer Do you have any idea how to tackle this?

benthayer commented 3 years ago

Hi @vmdhhh

I know that this behavior is common when a program doesn't accept stdin. Sometimes when I open a terminal and before the command line shows up, I can type in characters that end up displayed on the command line like you have in that picture. I think this happens whenever a program doesn't read from stdin. If you read the characters, they could possibly get removed from stdin and wouldn't show up on the command line after execution is done. If the characters still show up, you could maybe use some control characters to remove them, but then you still might be able to see the typed characters for a short while.

I hope this helps. If you want, you can make a pull request and I can take a look at your code too

vmdhhh commented 3 years ago

Hi @benthayer I've created a PR #336 . Please take a look.