feat: Include Confirmation dialogue if a file is about to be overwritten

prithvijj commented 5 months ago

Changes

Added in a huh.NewConfirm and huh.NewInput fields, that checks if the output file already exists within the given directory
If the user selects to overwrite the file, execution continues as is
If the user selects to not overwrite the file, an input field is provided to select a new output filename
The huh forms will be rendered only if the stdout is a terminal

Testing Notes

Ran the following command go run ./... --execute "cat SECURITY.md"

Choosing Yes will overwrite to the default output filename

Ran the following command go run ./... --execute "cat SECURITY.md" again
Choosing No will prompt an Input field to enter the new output filename

After inputting a new filename shows that it works as expected

Other Notes

[x] I'm not sure how to fix the tests, since upon running go test -v ./... , the tests would hang, since it's probably expecting an input for the huh forms introduced. Would like to get some advice, shall ask on Discord too!
[x] Let me know if there is anything else I'm missing! I did check the CONTRIBUTING.md and the issue I wanted to help out on https://github.com/charmbracelet/freeze/issues/35

debdutdeb commented 5 months ago

I was going to look into adding this :+1: thanks !

maaslalani commented 5 months ago

Nice! I like this idea!

prithvijj commented 5 months ago

Changes:

Based on the Discord Conversation in the #contributions channel, ended up using the isatty lib to render the huh form, such that when the unit tests are run, it doesn't affect it

go test -v ./...

Also changed it from draft to ready for review!
Just have that one review comment i've responded to https://github.com/charmbracelet/freeze/pull/86#discussion_r1582604002,

but yeah, Lemme know if there's anything else missing

prithvijj commented 5 months ago

Changes:

Added in check if the new filename provided is a directory,
If it's a directory then throw err, else continue as is

Screenshot showing that a random directory is created beforehand and freeze is executed, which shows the overwrite confirmation form

Providing input this-is-dir to show that it is overwriting to a directory

Throws error indicating that it could not overwrite to the directory, which is expected

pbj@pop-os:~/Github/freeze2$ mkdir this-is-dir
pbj@pop-os:~/Github/freeze2$ go run ./... --execute "cat SECURITY.md"

   ERROR  'this-is-dir' is a directory. Hence cannot overwrite to the given filename

  could not overwrite to filename

exit status 1

Looking for a re-review!

prithvijj commented 4 months ago

Just been having discussions on Discord Channel for #contributions, been a bit stale on my end, There was a good point made about figuring out if a flag could be introduced which overwrites the file in case no tty is recognized, and I've responded with it

Let's see what that looks like, shall keep the PR updated

charmbracelet / freeze