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

[prithvijj]

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

• 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!
• 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 feat: include confirmation dialogue if a file is about to get overwritten #35

[debdutdeb]

I was going to look into adding this 👍 thanks !

[debdutdeb]

should check for if is a directory or not first. If directory, might be better to fail immediately. to save people from themselves.

[prithvijj]

Fair, I shall get to this in like the next 6 hours or something

[prithvijj]

Hey @debdutdeb

So I tried the following:

• freeze.png already exist
• Ran the following command below
• Entered the new file name as non-exist/file.png indicating an example of the directory not existing, and adding a file within a non existent directory
  And it failed which is good

go run ./... --execute "cat SECURITY.md"
                                      
   ERROR  Unable to convert SVG to PNG
                                      
  open non-exist/file.png: no such file or directory
                                                    
exit status 1

Obviously when I added it as a parameter for the output, it fails as expected since the file is being added within a non existent directory, which I think is intended

go run ./... --execute "cat SECURITY.md" --output "non-exist/file.png"
                                      
   ERROR  Unable to convert SVG to PNG
                                      
  open non-exist/file.png: no such file or directory
                                                    
exit status 1

And finally, when the form pops up and add the filename as non-exist-dir/non-exist-dir2 to also indicate 2 non existent directory, it fails as expected (yeah even though the non-exist-dir2 would end up becoming a file atleast with me being on Pop OS)

go run ./... --execute "cat SECURITY.md"
                                
   ERROR  Unable to write output
                                
  open non-exist-dir/non-exist-dir2: no such file or directory
                                                              
exit status 1

but yeah should I still go ahead and check if the given path is a directory? To me it seems like it ends up handling it, open to whatever!

[debdutdeb]

It does but ig that depends on how file is being written and expection for actions after the confirmation. Like code that writes to the files consider them simply files, and might someday want to remove before writing, which would remove the directory not knowing it is a directory.

So I'd suggest still adding that check.

[prithvijj]

Sure thing, will look into it within like 24 hours or something!

[prithvijj]

#86 (comment)

Gone ahead and resolved it here, lemme know if there's anything else!

[maaslalani]

Nice! I like this idea!

[prithvijj]

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 feat: Include Confirmation dialogue if a file is about to be overwritten #86 (comment),

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

[prithvijj]

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]

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