dots-hyprland icon indicating copy to clipboard operation
dots-hyprland copied to clipboard

[Feature] Added Update-dots.sh script

Open H0mire opened this issue 1 year ago • 7 comments

Automatically updates the Repository and the dots: Replaces by default .config and .local files in the home directory. Checks if user has modified files => Asking before replacing modified files. You are able to exclude folders from updating. e.g. the "custom" folder Implemented with Checksums.

H0mire avatar May 03 '24 17:05 H0mire

Generally this script choose the files under $base which does not have a exact same file with md5 hash under $home, and prompt whether to sync them, right?

I won't talk about the problem that you did not use set -e and either mkdir -p or equivalent, but the key point is that:

  1. All this can be done by using rsync with --checksum. You don't have to compare md5sum manually at all.
  2. This script only copies the modified files, or not. (Note that you did not ever consider the situation if the destination file does not exists.) It's even meaningless to continue the script after a "n" is given to "Do you want to keep these files untouched? [Y/n]" because it won't do anything anyway, just exit.

~~Though I guess it's possible to implement similar function for ./install.sh, such as using --no-overwrite. I'll work on it when I'm available.~~

~~Thanks for your contribution anyway.~~

~~Closing.~~


My apologies, I rechecked the logic and I've found that the position of git pull matters.

So, if a file is different with the destination before git pull, the script assume it's "modified", which means it may contain some useful changes from the user, so user should be allowed to skip such file after git pull and sync them.

The problem is that the script is not robust, such as:

  • This assumption about "modified" only holds true when "all files were synchronized the last time the user used install.sh, and no methods like git pull were used to update this repository after that till now."
    • Also there are some files which will be modified by .config/ags/scripts/color_generation/applycolor.sh, not by users.
    • You'd better let user to choose which ones of them, not only a [y/n] for all of them or none of them.
  • git pull may fail when a force push has happened in remote repo.
  • If something went wrong, you need a set -e at the beginning to make the script abort.
  • You did not consider the situation when the destination file does not exists.
  • You did not consider the situation when the destination folder structure does not exists.

And for the exclusion paths, you only considered .config/hypr/custom but .config/ags/user_options.js and .config/hypr/hyprland.conf should also be excluded. Note that they are files, not folders.

If you can solve these problem, I think it's OK to be merged.

clsty avatar May 03 '24 23:05 clsty

Thank you for your feedback.

  • Correct, i considered rsync, but as you know it would remove the user modified scripts, since the new ones have a different checksum and metadata.
  • ~I also considered the case that git pull has already been called. In that case, the scripts tells the user, that no file has been found that has been configured by the user, and asks if every file should be replaced.~ In order to make it independent there must be some cache that holds the checksums created with the install.sh script or the update-dots script. Should I implement that?
  • Ok, I will add the possibility to decide for each file independently. But i still will leave a -all option.
  • The rest will be fixed.

H0mire avatar May 04 '24 07:05 H0mire

@clsty I updated the script. Could you please review the script again?

Thanks!

H0mire avatar May 04 '24 08:05 H0mire

The git clone part has some major problems.

  • I suggest to use a --depth=1 for git clone.
  • find "$temp_folder/$folder" -type f -print0 | while IFS= read -r -d '' file; do will give you a $file var which looks like ./cache/tmp.rhqpeLo6Zw/.config/ags/..., you surely don't want that ./cache/tmp.rhqpeLo6Zw thing, else $HOME/$file will be messed up.
  • It lacks logic to skip modified files.
  • After this part is done and temp directory removed, the script does not stop, but continues to copy files from main directory again.

Another problem is that you need to prompt user what this script is for, and give user chance to exit in case they run this by accident.


As for the rest part, after reading through it, it seems OK to me, but I don't have a suitable environment to test it, so I can't tell if there're more problems.

Nor have you tested it, right? Else I would not have found out the major problems above.

I suggest you to use it by yourself for a couple of days so you will be able to fix/improve some neglected things, if there is any.


@end-4 What do you think about this script?

clsty avatar May 04 '24 12:05 clsty

Good job, great awareness. Makes sense. I tested it, but seems like the wrong way, lol. Thanks, working on it...

H0mire avatar May 04 '24 14:05 H0mire

Ordered, delivered. Can you check? @end-4 and @clsty

Update: Tested now by resetting to an earlier commit. Tested Cases:

  • Git pull working, with and without checking files individually
  • Git pull not working, with and without checking files individually

H0mire avatar May 04 '24 15:05 H0mire

@end-4 What do you think about this script?

@clsty I don't see any problem Just gonna comment that the grammar/wording is good xd Since this is a scripting-related PR I'll let you decide when to merge

end-4 avatar May 04 '24 17:05 end-4

@clsty It's been a while. Anything that must be improved? Encountered no error since.

H0mire avatar May 16 '24 11:05 H0mire

I can't find out any problem for now, so I'll merge it. Thank you for contribution.

clsty avatar May 16 '24 22:05 clsty