0

I seem to have this problem. This code breaks at line 119 in my script with bash associative arrays. I am sorry for the comments but I am kind to new to bash scripting. This is the code:

#!/bin/bash
# Aliases file

# Command usage: cpRecent/mvRecent -d {dirFrom},{dirTo} -n {numberofFiles} -e {editTheNames}

# Error codes
NO_ARGS="You need to pass in an argument"
INVALID_OPTION="Invaild option:"
NO_DIRECTORY="No directory found"


# Return values 
fullpath=
directories=
numfiles=
interactive=

typeset -a files
typeset -A filelist

# Advise that you use relative paths
__returnFullPath(){
    local npath
    if [[ -d $1 ]]; then
        cd "$(dirname $1)"
        npath="$PWD/$(basename $1)"
        npath="$npath/"         #Add a slash
        npath="${npath%.*}"     #Delete . 
    fi
    fullpath=${npath:=""}
}

__usage(){

wall <<End-Of-Message
________________________________________________
<cpRecent/mvRecent> -d "<d1>,<d2>" -n <num> [-i]
    -d First flag: Takes two arguments 
    -n Second flag: Takes one argument 
    -i Takes no arguments. Interactive mode
    d1 Directory we are reading from    
    d2 Directory we are writing to
    num Number of files 
________________________________________________
End-Of-Message
}


__processOptions(){
    while getopts ":d:n:i" opt; do
        case $opt in 
            d ) IFS=',' read -r -a directories <<< "$OPTARG";; 
            n ) numfiles=$OPTARG;;
            i ) interactive=1;;
            \? ) echo "$INVALID_OPTION -$OPTARG" >&2 ; return 1;;
            : ) echo "$NO_ARGS"; __usage; return 1;;
            * ) __usage; return 1;;
        esac
    done            
}


__getRecentFiles(){

    # Check some conditions
    (( ${#directories[@]} != 2 )) && echo "$INVALID_OPTION Number of directories must be 2" && return 2
    #echo ${directories[0]} ${directories[1]}

    # Get the full paths of the directories to be read from/written to
    __returnFullPath "${directories[0]}"
    directories[0]="$fullpath"
    __returnFullPath "${directories[1]}"
    directories[1]="$fullpath"  

    if [[ -z ${directories[0]} || -z ${directories[1]} ]]; then
        echo $NO_DIRECTORY 
        return 3
    fi

    [[ numfiles != *[!0-9]* ]] && echo "$INVALID_OPTION Number of files cannot be a string" && return 4

    #numfiles=$(($numfiles + 0))
    (( $numfiles == 0 )) && echo "$INVALID_OPTION Number of files cannot be zero" && return 4

    local num="-"$numfiles""

    # Get the requested files in directory(skips directories)
    if [[ -n "$(ls -t ${directories[0]} | head $num)" ]]; then
        # For some reason using local -a or declare -a does not seem to split the string into two
        local tempfiles=($(ls -t ${directories[0]} | head $num))
        #IFS=' ' read -r -a tempfiles <<< "$string" 
        #echo ${tempfiles[@]}
        for index in "${!tempfiles[@]}"; do
            echo $index ${tempfiles[index]}
            [[ -f "${directories[0]}${tempfiles[index]}" ]] && files+=("${tempfiles[index]}") 
        done
    fi

}

####################################
# The problem is this piece of code
__processLines(){
    local name
    local answer
    local dirFrom
    local dirTo
    if [[ -n $interactive ]]; then
        for (( i=0; i< ${#files[@]}; i++ )); do
            name=${files[i]}
            read -n 1 -p "Old name: $name. Do you wish to change the name(y/n)?" answer
            [[ answer="y" ]] && read -p "Enter new name:" name
            dirFrom="${directories[0]}${files[i]}"
            dirTo="${directories[1]}$name"
            fileslist["$dirFrom"]="$dirTo"
        done
    else                                    
        for line in $files; do
            dirFrom="${directories[0]}$line"
            echo $dirFrom # => /home/reclusiarch/Documents/test
            dirTo="${directories[1]}$line"
            echo $dirTo # => /home/reclusiarch/test
            fileslist["$dirFrom"]="$dirTo" # This is the offending line
        done    
    fi

}
###########################################################

cpRecent(){
    __processOptions $*
    __getRecentFiles
    __processLines
    for line in "${!filelist[@]}"; do
        cp $line ${filelist[$line]}
    done
    echo "You have copied ${#fileList[@]} files"
    unset files
    unset filelist
    return
}

mvRecent(){
    __processOptions $*
    __getRecentFiles
    __processLines
    for line in "${!filelist[@]}"; do
        mv $line ${filelist[$line]}
    done
    echo "You have copied ${#fileList[@]} files"
    unset files
    unset filelist
    return
}

cpRecent "$*"

I have tried a lot of things. To run the script,

$ bash -x ./testing.sh -d "Documents,." -n 2

But nothing seems to work: The error is this(when using bash -x):

./testing.sh: line 119: /home/reclusiarch/Documents/test: syntax error: operand expected (error token is "/home/reclusiarch/Documents/test")

If I run that section on the command line, it works:

$ typeset -A filelist
$ filelist["/home/reclusiarch/Documents/test"]=/home/reclusiarch/test
$ echo ${filelist["/home/reclusiarch/Documents/test"]}
/home/reclusiarch/test

Thanks for your help!!

Edit: I intially pared down the script to the piece of offending code but that might make it not run. Again, if you want to test it, you could run the bash command given. (The script ideally would reside in the user's $HOME directory).

Edit: Solved (Charles Duffy solved it) It was a simple mistake of forgetting which name was which.

corax
  • 193
  • 1
  • 4
  • 17
  • 2
    You need to pare this down to a *minimum* example; most of the code you have posted is irrelevant to your problem. – chepner Feb 27 '17 at 22:10
  • How do you launch your script, can you put command line? Anyway, @chepner is right, try to reduce to minimum needed expression! – OscarAkaElvis Feb 27 '17 at 22:12
  • 1
    And really, `wall` to display the usage message? Other users of the system are not going to appreciate hearing about how one person misused your script. – chepner Feb 27 '17 at 22:16
  • @chepner I have pared it down. But the other pieces of code enable the script to run. I might as well add it in. – corax Feb 27 '17 at 22:31
  • 1
    BTW, using `$*` (which munges all your entries into a big string, destroying information about their original boundaries) is a code smell -- *generally*, the right way to pass or handle any array is keeping its elements distinct, with `"$@"`. Consider using http://shellcheck.net/ habitually. – Charles Duffy Feb 27 '17 at 22:42
  • Re: paring things down correctly -- see http://stackoverflow.com/help/mcve. You're expected to do *both* things -- to make your example small, **and** to make it runnable (ensuring that it genuinely reproduces the error at hand). – Charles Duffy Feb 27 '17 at 22:43
  • @OscarAkaElvis Charles helped me see my mistake. Thanks. – corax Feb 27 '17 at 22:47
  • @chepner Is using wall a code smell? How do I make the script self-documenting apart from the obvious(descriptive names, proper formatting, etc)? Thanks. – corax Feb 27 '17 at 22:48
  • @corax, `wall` writes to *all* users connected to your system. If you want to write only to the user who started your script, consider stderr (ie. `echo "this is an error" >&2`). – Charles Duffy Feb 27 '17 at 22:49
  • You may want to look at [Correct Bash and shell script variable capitalization](http://stackoverflow.com/questions/673055/correct-bash-and-shell-script-variable-capitalization) – codeforester Feb 28 '17 at 00:01
  • @codeforester Does this also include variables that contain error codes? – corax Feb 28 '17 at 00:42

1 Answers1

0

Your declaration is:

typeset -A filelist

However, your usage is:

fileslist["$dirFrom"]="$dirTo"

fileslist is not filelist.

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
  • I feel so stupid. I guess I was too tired to look at it. Damm. Thanks for seeing it. BTW, would "$@" allow me to using getopt instead of "$*"? – corax Feb 27 '17 at 22:46
  • Yes, but `getopt` is actually bad practice. See [BashFAQ #35](http://mywiki.wooledge.org/BashFAQ/035) for robust alternatives. – Charles Duffy Feb 27 '17 at 22:47
  • Thanks. Looking at the link now. – corax Feb 27 '17 at 22:50