Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor script to use functions #3

Merged
merged 2 commits into from
Oct 24, 2021
Merged

Conversation

Mudskipper875
Copy link
Contributor

Added two functions: One for moving/backing up the dotfiles and one for appending
the 'export variable' command to tidytilde_backups.

The functions are explained in the comments.

I'm not sure if backups for all the dotfiles are needed. Some directories like .rustup and .cargo are very big and will take a while to copy.

@gtlsgamr
Copy link
Owner

The backups are necessary, at least until we have tested this script to hell and back. Looks fine otherwise.

@Mudskipper875
Copy link
Contributor Author

Mudskipper875 commented Oct 23, 2021

The backups are necessary, at least until we have tested this script to hell and back. Looks fine otherwise.

I removed the copying/backups for now. Since all the files are just being moved, it will be more efficient if we
could just move them back to the original location (with exceptions like .fehbg which gets removed).

This can probably be implemented the same way in which something like trash-cli is able to restore the trashed files
to it's original location.

(force push 2: Added a fix to delete directories only if the contents were moved)

Added two functions for moving the dotfiles and appending the
'export variable' command to tidytilde_backups.

The functions are explained in the comments.
tidytilde Outdated
if [[ "$1" = "-a" ]]; then
shift
printf '\tAlias needs to be set. '
echo "alias $1='$2'" >> "$TIDYTILDE_COMMANDS"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use

printf '%s\n' "alias $1='$2'" >>

If $2 contained a \n then echo would print a new line there

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly to all other echo's with variables

tidytilde Outdated
if [[ "$1" = "-a" ]]; then
shift
printf '\tAlias needs to be set. '
echo "alias $1='$2'" >> "$TIDYTILDE_COMMANDS"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly to all other echo's with variables

tidytilde Outdated
# exists, 'mv -i' will prompt the user for overwriting. If "$1" is a directory
# and the destination is an already existing dirctory, it will move the
# contents of the dotdir into the existing destination directory. If only "$1"
# is passed, it will remove it from $HOME.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if only $1 being passed removing seems a little confusing. It may lead to some confusions. Maybe it would be better to have a remove_file function

tidytilde Outdated
if
[[ -d "$destination" ]]
then
[[ -n "$(ls -A "$dotfile")" ]] && mv -i "$dotfile"/* "$destination" || printf '\tRemoving empty directory\n'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we are using bash, look at the nullglob shopt built in

shopt -s nullglob
files=( "$dotfile"/* )
if (( ${#files[@]} == 0 )); then
     mv ....
fi

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null glob returns no objects if glob fails to match anyfile

Add a separate function for removing files. Use `$#` to test the number
of arguments in a function. Replace `echo` with `printf`. Use `nullglob`
to determine if directory is empty before removing it.
@Mudskipper875
Copy link
Contributor Author

I've made the suggested changes.

@gtlsgamr gtlsgamr merged commit 9688bd1 into gtlsgamr:main Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants