Tuckr icon indicating copy to clipboard operation
Tuckr copied to clipboard

Follow-up fixes for #112

Open MikeTheGreat opened this issue 1 month ago • 1 comments

I saw your commit for #112. It looks good and works well! There's a couple small things that I'm submitting this PR for.

I checked (and confirmed to be working):

1: from-stow wanted to convert the stow directory into a temporary directory but it failed because it never created the temp dir ; now it does 2: refactor help text so that it shows up when you run tuckr -h from-stow (not just when you run the command itself)

Minor errors:

3: if dotfiles_old already exists we exit with a clear message saying this (instead of an error when we try to rename the existing dotfiles dir)

The code is checking if the dotfiles directory (not dotfiles_old) exists and stopping at that point. My PR fixes this so that it'll print an error only when the _old directory exists. This means that if the dotfiles dir is there but not the _old dir then from-stow will move the current dotfiles dir into the backup _old dir

4: misc, minor adjustments to dry-run - tidying up messages, and also prevent a failure when the new, temp dotfiles dir wasn't created

i didn't verify this works by running it, but looking at the code there needs to be an 'else' for if file.is_dir() { so that we don't try to create the dir in dry-run mode.

New items:

5: If something goes wrong with the fs::copy(&file, &tuckr_path) operation the whole thing will halt immediately - it would be nice for this to print an error message and then keep going. This is probably a corner case, but I encountered it when I had a broken link in my stow-files dir. The real error is that I had a link in my stow-files dir, but having a nice error message would help folks figure that out.

6: This is super-minor, but the code always tell the user that the old dotfiles are available in dotfiles_old, even if we didn't create an _old directory.

One last request: could you accept my PR into the project? I'd really like to be able to brag about "I made a commit".

MikeTheGreat avatar Nov 08 '25 21:11 MikeTheGreat

Hi, I'm just letting you know that I'm busy right now so I haven't had time to go and review PRs for merging. I should have the time sometime between now and the weekend. We'll see.

RaphGL avatar Nov 11 '25 10:11 RaphGL

I've read your code but half of it doesn't make sense. Let me comment on the actual code.

RaphGL avatar Nov 16 '25 11:11 RaphGL

2: refactor help text so that it shows up when you run tuckr -h from-stow (not just when you run the command itself)

You didn't actually do this and I think it also wouldn't be possible to do this and still make the text colored? This is heavily dependent on what rust allows to be in a const context. So I removed it while basing my previous fix on yours.

RaphGL avatar Nov 16 '25 11:11 RaphGL

2: refactor help text so that it shows up when you run tuckr -h from-stow (not just when you run the command itself)

You didn't actually do this and I think it also wouldn't be possible to do this and still make the text colored? This is heavily dependent on what rust allows to be in a const context. So I removed it while basing my previous fix on yours.

I'm sorry for not being more clear - items #1 and #2 on my list (at the top of this thread) was me testing your code commit for #112, as per your request to "Test it out and let me know if something comes up". Your code does print out the entire message as the "long_about" for the from_stow_cmd (which is great!) and also prints out the message, with the yellow color, when the user runs the from_stow_cmd.

So yeah - this was me trying to say that your code is awesome! 😁👍

MikeTheGreat avatar Nov 17 '25 02:11 MikeTheGreat