mfhepp / cdf

cdf: Change to favorite directories in Bash
https://mfhepp.github.io/cdf/
MIT License
1 stars 0 forks source link

Refactoring: Make code more robust and more portable #1

Closed mfhepp closed 7 months ago

mfhepp commented 7 months ago

The code could be improved to be more consistent and more portable (mostly ChatGPT 4 recommendations):

  1. Unify the use of echo vs. printf.
  2. Quote Variables: Always quote your variables to handle spaces and special characters correctly. For instance, use "$PWD" instead of $PWD etc.
  3. Consistent if Statement Syntax: For portability and readability, use a consistent style for if statements. For example, instead of [[ "$CURRENT_DIR" = "$CDFPATH" || "$CURRENT_DIR" == "$CDFPATH"/ ]], use [ "$CURRENT_DIR" = "$CDFPATH" ] || [ "$CURRENT_DIR" = "$CDFPATH"/ ].
  4. Avoid Using [[ for Portability: The [[ syntax is not POSIX-compliant and might not work in shells other than Bash and Zsh. Use [ or test for portability.
  5. Handle realpath and ln Commands for Portability: The realpath and ln commands might not be available or behave differently on some systems. You can write a function to manually resolve real paths or use readlink as an alternative. Similarly, for ln, check if the command exists and handle errors.
  6. Handle cd Command Failure: The cd command can fail (e.g., due to permission issues). Always check its exit status to ensure the subsequent commands operate in the correct directory.
  7. Avoid Using find for Autocomplete: The find command in _cdf_autocomplete may not be efficient or necessary. Consider using a simpler approach, like a for loop over the contents of $CDFPATH.
  8. Sanitize Input: Ensure that input (like shortcut names) is sanitized to prevent potential command injection or other security issues or conflicts. Maybe simply restrict the format of the shortcut name via regexes.
  9. Check for Command Existence: Before using commands like find, realpath, or ln, check if they exist on the system for better portability.
  10. Test with zsh and other Non-Bash shells.
mfhepp commented 7 months ago
  1. Unify the use of echo vs. printf.

done, except for tests where it's not worth the effort.

  1. Quote Variables: Always quote your variables to handle spaces and special characters correctly. For instance, use "$PWD" instead of $PWD etc.

done, except maybe for tests

  1. Handle realpath and ln Commands for Portability: The realpath and ln commands might not be available or behave differently on some systems. You can write a function to manually resolve real paths or use readlink as an alternative. Similarly, for ln, check if the command exists and handle errors.

done in the form of a test at install time and run time. Likely available on most modern systems.

  1. Handle cd Command Failure: The cd command can fail (e.g., due to permission issues). Always check its exit status to ensure the subsequent commands operate in the correct directory.

done, same for ln in addfav

  1. Sanitize Input: Ensure that input (like shortcut names) is sanitized to prevent potential command injection or other security issues or conflicts. Maybe simply restrict the format of the shortcut name via regexes.

done for the shortcut name in both cdf and addfav

  1. Check for Command Existence: Before using commands like find, realpath, or ln, check if they exist on the system for better portability.

done, see No. 5, except for find (which may be replaceable by a loop or similar)

mfhepp commented 7 months ago

TODO:

  1. Consistent if Statement Syntax: For portability and readability, use a consistent style for if statements. For example, instead of [[ "$CURRENT_DIR" = "$CDFPATH" || "$CURRENT_DIR" == "$CDFPATH"/ ]], use [ "$CURRENT_DIR" = "$CDFPATH" ] || [ "$CURRENT_DIR" = "$CDFPATH"/ ].
  2. Avoid Using [[ for Portability: The [[ syntax is not POSIX-compliant and might not work in shells other than Bash and Zsh. Use [ or test for portability.
  3. Avoid Using find for Autocomplete: The find command in _cdf_autocomplete may not be efficient or necessary. Consider using a simpler approach, like a for loop over the contents of $CDFPATH.
  4. Test with zsh and other Non-Bash shells.
mfhepp commented 7 months ago

The check for find is now also implemented.

mfhepp commented 7 months ago

Completed with v0.4