rm.sh, test: fixes linux trash, fixes duplicate files in trash for bo…

…th macos and linux

README.md

@@ -14,15 +14,14 @@
 
 A much safer replacement of bash `rm` with **ALMOST FULL** features of the origin `rm` command.
 
-Initially developed on Mac OS X, then tested on Linux.
-
-Using `safe-rm`, the files or directories you choose to remove will move to `$HOME/.Trash` instead of simply deleting them. You could put them back whenever you want manually.
-
-If a file or directory with the same name already exists in the Trash, the name of newly-deleted items will be ended with the current date and time.
+The project was initially developed for Mac OS X, and then tested on Linux.
 
 ## Features
-- On MacOS, safe-rm` will use AppleScript to delete files or directories as much as possible to enable the built-in "put-back" capability in the system Trash bin.
-- Custom [configurations](#configuration).
+- Supports both MacOS and Linux with full test coverage.
+- Using `safe-rm`, the files or directories you choose to remove will be moved to the system Trash instead of simply deleting them. You could put them back whenever you want manually.
+  - On MacOS, `safe-rm` will use [AppleScript][applescript] to delete files or directories as much as possible to enable the built-in "put-back" capability in the system Trash bin.
+  - On Linux, t also follows the operating system's conventions for handling duplicate files in the Trash to avoid overwriting
+- Supports Custom [configurations](#configuration).
 
 ## Supported options
 
@@ -56,22 +55,22 @@ and `/path/to` is where you git clone `shell-safe-rm` in your local machine.
 
 ## Permanent Installation
 
-If you have NPM (node) installed (RECOMMENDED):
+If you have NPM ([NodeJS](https://nodejs.org/)) installed (RECOMMENDED):
 
 ```sh
 npm i -g safe-rm
 ```
 
-Or normally with `make` (not recommended, may be unstable):
+Or by using the source code, within the root of the current repo (not recommended, may be unstable):
 
 ```sh
-make && sudo make install
-# and enjoy
-```
+# If you have NodeJS installed
+npm link
 
-For those who have no `make` command:
+# If you don't have NodeJS or npm installed
+make && sudo make install
 
-```sh
+# For those who have no `make` command:
 sudo sh install.sh
 ```
 
@@ -121,7 +120,7 @@ sudo sh uninstall.sh
 Since 2.0.0, you could create a configuration file named `.safe-rm.conf` in your HOME directory, to support
 - defining your custom trash directory
 - allowing `safe-rm` to permanently delete files and directories that are already in the trash
-- disallowing `safe-rm` to use [AppleScript](https://en.wikipedia.org/wiki/AppleScript)
+- disallowing `safe-rm` to use [AppleScript][applescript]
 
 For the description of each config, you could refer to the sample file [here](./sample.safe-rm.conf)
 
@@ -136,3 +135,6 @@ Or if it is installed by npm:
 ```sh
 alias="SAFE_RM_CONF=/path/to/safe-rm.conf safe-rm"
 ```
+
+
+[applescript]: https://en.wikipedia.org/wiki/AppleScript

bin/rm.sh

@@ -61,21 +61,14 @@ do_exit(){
 }
 
 
-OS="$(uname -s)"
-
-case "$OS" in
-    Darwin*)
-      OS_TYPE="MacOS"
-      DEFAULT_TRASH="$HOME/.Trash"
-      ;;
-
-    # We treat all other systems as Linux
-    *)
-      OS_TYPE="Linux"
-      DEFAULT_TRASH="$HOME/.local/share/Trash"
-      SAFE_RM_USE_APPLESCRIPT=
-      ;;
-esac
+if [[ "$(uname -s)" == "Darwin"* && -z $SAFE_RM_DEBUG_LINUX ]]; then
+  OS_TYPE="MacOS"
+  DEFAULT_TRASH="$HOME/.Trash"
+else
+  OS_TYPE="Linux"
+  DEFAULT_TRASH="$HOME/.local/share/Trash"
+  SAFE_RM_USE_APPLESCRIPT=
+fi
 
 
 # The target trash directory to dispose files and directories,
@@ -108,6 +101,13 @@ else
     error "$COMMAND: failed to create trash directory $SAFE_RM_TRASH/files"
     do_exit $LINENO 1
   fi
+
+  if mkdir -p "$SAFE_RM_TRASH/info" &> /dev/null; then
+    :
+  else
+    error "$COMMAND: failed to create trash info directory $SAFE_RM_TRASH/info"
+    do_exit $LINENO 1
+  fi
 fi
 
 # ------------------------------------------------------------------------------
@@ -431,17 +431,18 @@ short_time(){
 _mac_trash_path_ret=
 check_mac_trash_path(){
   local path=$1
+  local ext=$2
+  local full_path="$path$ext"
 
   # if already in the trash
-  if [[ -e "$path" ]]; then
-    debug "$LINENO: $path already exists"
+  if [[ -e "$full_path" ]]; then
+    debug "$LINENO: $full_path already exists"
 
     # renew $_short_time_ret
     short_time
-    _mac_trash_path_ret="$path $_short_time_ret"
-    check_mac_trash_path "$_mac_trash_path_ret"
+    check_mac_trash_path "$path $_short_time_ret" "$ext"
   else
-    _mac_trash_path_ret=$path
+    _mac_trash_path_ret=$full_path
   fi
 }
 
@@ -481,7 +482,21 @@ mac_trash(){
   local move=$_to_move
   local base=$(basename "$move")
 
-  check_mac_trash_path "$SAFE_RM_TRASH/$base"
+  # foo.jpg => "foo" + ".jpg"
+  # foo => "foo" + ""
+
+  local name="${base%.*}"
+  local ext="${base##*.}"
+
+  if [[ "$name" == "$ext" ]]; then
+    ext=
+  else
+    ext=".$ext"
+  fi
+
+  # foo.jpg => "foo 12.34.56.jpg"
+
+  check_mac_trash_path "$SAFE_RM_TRASH/$name" "$ext"
   local trash_path=$_mac_trash_path_ret
 
   [[ "$OPT_VERBOSE" == 1 ]] && list_files "$1"
@@ -496,7 +511,6 @@ mac_trash(){
 }
 
 
-_linux_trash_base_ret=
 check_linux_trash_base(){
   local base=$1
   local trash="$SAFE_RM_TRASH/files"
@@ -506,16 +520,24 @@ check_linux_trash_base(){
   if [[ -e "$path" ]]; then
     debug "$LINENO: $path already exists"
 
-    max_n=$(find "$trash" -type f -name "${base}.*" \
-    | grep -oP "${base}\.\K\d+" \
-    | sort -n \
-    | tail -1)
+    local max_n=0
+    local num=
+
+    while IFS= read -r file; do
+      if [[ $file =~ ${filename}\.([0-9]+)$ ]]; then
+          # Remove leading zeros and make sure the number is in base 10
+          num=$((10#${BASH_REMATCH[1]}))
+          if ((num > max_n)); then
+              max_n=$num
+          fi
+      fi
+    done < <(find "$trash" -maxdepth 1)
 
     (( max_n += 1 ))
 
-    _linux_trash_base_ret="$base.$max_n"
+    echo "$base.$max_n"
   else
-    _linux_trash_base_ret=$base
+    echo "$base"
   fi
 }
 
@@ -527,8 +549,7 @@ linux_trash(){
   local move=$_to_move
   local base=$(basename "$move")
 
-  check_linux_trash_base "$base"
-  base=$_linux_trash_base_ret
+  base=$(check_linux_trash_base "$base")
 
   local trash_path="$SAFE_RM_TRASH/files/$base"
 

package.json

@@ -9,6 +9,8 @@
     "test": "ava --timeout=10s --verbose",
     "test:dev": "NODE_DEBUG=safe-rm npm run test",
     "test:debug": "SAFE_RM_DEBUG=1 npm run test:dev",
+    "test:mock-linux": "SAFE_RM_DEBUG_LINUX=1 npm run test",
+    "test:mock-linux:debug": "SAFE_RM_DEBUG=1 SAFE_RM_DEBUG_LINUX=1 npm run test:dev",
     "lint": "eslint .",
     "fix": "eslint . --fix"
   },

test/cases.js

@@ -4,20 +4,19 @@ const delay = require('delay')
 
 const {
   generateContextMethods,
-  assertEmptySuccess
+  assertEmptySuccess,
+  IS_MACOS
 } = require('./helper')
 
 
 module.exports = (
   test,
   des_prefix,
-  test_safe_rm = true,
+  test_trash_dir = true,
   rm_command,
   env = {}
 ) => {
 
-  const IS_MACOS = process.platform === 'darwin'
-
   // Setup before each test
   test.beforeEach(generateContextMethods(rm_command, env))
 
@@ -35,7 +34,7 @@ module.exports = (
     t.is(result.code, 0, 'exit code should be 0')
     t.false(await pathExists(filepath), 'file should be removed')
 
-    if (!test_safe_rm) {
+    if (!test_trash_dir) {
       return
     }
 
@@ -64,27 +63,31 @@ module.exports = (
 
     const filepath1 = await createFile(filename, '1')
     const result1 = await runRm([filepath1])
-    assertEmptySuccess(t, result1)
-    t.false(await pathExists(filepath1), 'file 1 should be removed')
 
     const filepath2 = await createFile(filename, '2')
     const result2 = await runRm([filepath2])
-    assertEmptySuccess(t, result2)
-    t.false(await pathExists(filepath2), 'file 2 should be removed')
 
     const filepath3 = await createFile(filename, '3')
     const result3 = await runRm([filepath3])
+
+    assertEmptySuccess(t, result1)
+    t.false(await pathExists(filepath1), 'file 1 should be removed')
+
+    assertEmptySuccess(t, result2)
+    t.false(await pathExists(filepath2), 'file 2 should be removed')
+
     assertEmptySuccess(t, result3)
     t.false(await pathExists(filepath3), 'file 3 should be removed')
 
-    if (!test_safe_rm) {
+    if (!test_trash_dir) {
       return
     }
 
     // /path/to/foo
     // /path/to/foo 12.58.23
     // /path/to/foo 12.58.23 12.58.23
     const [f1, f2, f3] = (await lsFileInTrash(filename))
+
     const [fb1, fb2, fb3] = [f1, f2, f3].map(f => path.basename(f))
 
     if (IS_MACOS) {
@@ -122,7 +125,7 @@ module.exports = (
     assertEmptySuccess(t, result)
     t.false(await pathExists(filepath), 'file should be removed')
 
-    if (!test_safe_rm) {
+    if (!test_trash_dir) {
       return
     }
 

test/helper.js

@@ -9,6 +9,10 @@ const log = require('util').debuglog('safe-rm')
 const SAFE_RM_PATH = path.join(__dirname, '..', 'bin', 'rm.sh')
 const TEST_DIR = path.join(tmp.dirSync().name, 'safe-rm-tests')
 
+const IS_MACOS = process.platform === 'darwin'
+  // For linux mock testing
+  && !process.env.SAFE_RM_DEBUG_LINUX
+
 const generateContextMethods = (
   rm_command = SAFE_RM_PATH,
   rm_command_env = {}
@@ -111,18 +115,44 @@ const generateContextMethods = (
     }
   }
 
-  async function lsFileInTrash (filepath) {
+
+  async function lsFileInMacTrash (filepath) {
     const {trash_path} = t.context
+
     const files = await fs.readdir(trash_path)
 
-    const filename = path.basename(filepath)
+    const _filename = path.basename(filepath)
+    const ext = path.extname(_filename)
+    const filename = path.basename(_filename, ext)
+
+    const regex = new RegExp(`${filename}.*${ext}$`)
+
     const filtered = files.filter(
-      f => f === filename || f.startsWith(filename)
+      f => f.endsWith(ext) && f.startsWith(filename)
     ).map(f => path.join(trash_path, f))
 
     return filtered
   }
 
+  async function lsFileInLinuxTrash (filepath) {
+    const {trash_path: _trash_path} = t.context
+    const trash_path = path.join(_trash_path, 'files')
+
+    const filename = path.basename(filepath)
+
+    const files = await fs.readdir(trash_path)
+
+    return files
+    .filter(f => f.startsWith(filename))
+    .map(f => path.join(trash_path, f))
+  }
+
+  async function lsFileInTrash (filepath) {
+    return IS_MACOS
+      ? lsFileInMacTrash(filepath)
+      : lsFileInLinuxTrash(filepath)
+  }
+
   Object.assign(t.context, {
     createDir,
     createFile,
@@ -140,5 +170,6 @@ const assertEmptySuccess = (t, result, a = '', b = '', c = '') => {
 
 module.exports = {
   generateContextMethods,
-  assertEmptySuccess
+  assertEmptySuccess,
+  IS_MACOS
 }