search

tj / git-extras

GIT utilities -- repo summary, repl, changelog population, author commit percentages and more
MIT License
17.37k stars 1.21k forks source link

Please add more safeguards to `git clear` #1167

Open Welding-Torch opened 1 month ago

Welding-Torch commented 1 month ago

Hi,

This issue is in regards to the git clear command.

I was showing git extras to some people today using the git repl. I typed clear to clear the text in the repl. It asked me if I was Sure. I thought it was asking me if I was sure that I wanted to clear my screen. I didn't think for more than a few seconds about it and I typed "y" for yes. It didn't clear my screen. What it actually did was hard delete 4 files worth of code that existed only in that directory with no other copies.

I do realise that this is my mistake and that I was in a rush, so I didn't pay enough attention.

However, I still want to urge @tj and team to add some more loud safeguards to git clear so that users understand that the action they are about to take is absolutely irreversible and will delete un-tracked code. I would suggest doing this by:

  1. Printing the line Sure? - This command may delete files that cannot be recovered, including those in .gitignore [y/N]: in CAPITAL and red color letters.
  2. Asking the user a second time just to make sure. And printing the list of files that are about to be affected by this command.

Thank you. I sincerely hope you act on this.

hyperupcall commented 1 month ago

Hello! Thanks for the report; I've made a PR that writes part of the warning in uppercase characters. I haven't implemented the red colors part and printing the list of files since that is slightly less straightfoward. (printing red colors should only be done conditionally (only if stdin is a tty), and that requires some common logic, which this repository isn't quite set up for). In the future, hopefully more can be done

vanpipy commented 1 month ago

There is a git-clear-soft and i think to do the more safeguards in it will be better. But I dont think to make more safeguards is a good way cause to use git reset --hard is the way user must know what happened when invoking it.

Welding-Torch commented 1 month ago

@hyperupcall Hi, thank you so much for responding to this issue and making a PR quickly. This PR is good, capital letters are a good start.

I haven't implemented the red colors part and printing the list of files since that is slightly less straightfoward.

I understand, you've tackled a part of the problem quickly. I'm hoping that the next PR will have asking the user to confirm (y/N) a second time, because that would also be quick and easy to implement.

The red colored text should first check if the terminal supports colored text and only then print it. And as for listing the files, git clear already prints the list of files after the command has been executed. So the logic needs to just be tweaked slightly to print the list of files affected before the command is finished, as seen in this image image

Hope this helps 👍

Welding-Torch commented 1 month ago

This might help as a reference for adding red colored text in the terminal: https://jvns.ca/blog/2024/10/01/terminal-colours/