2

I wrote a script in bash to basically create aliases, but for any reason, it doesn't work in some cases. Here it's the code:

#!/bin/bash
#Script to create unique and permanent aliases

_add_alias()
{
    echo "alias $1='$2'" | sudo tee -a $HOME/.bash_aliases
    echo "Alias successfully created"
}

if [ `cat $HOME/.bash_aliases | grep "alias $1=" | wc -l` == 0 ]
    then
    if [ $# -eq 2 ]
        then
        if [ -f "$2" ] #This condition is always false, don't know why.
            then
            _add_alias $1 "$2"
        elif $2 > tmp.txt 2> tmp.txt
            then
            _add_alias $1 "$2"
        else
            echo "Wrong command"
        fi
        rm tmp.txt
    elif [ $# -gt 2 ]
        then
    echo "Error: Usage: addal alias_name 'command' (comand between quotes)"
    elif [ $# -lt 2 ]
        then
        echo "Wrong number of parameters"
    fi
else
    echo "Alias name already exists"
fi

I hope you could help me fix the script.

0x2b3bfa0
  • 8,620
  • 5
  • 38
  • 59
Francisco Gallego Salido
  • 238
  • 1
  • 8
  • What cases does it work and when does it fail? – Oli Apr 14 '15 at 11:21
  • It doesn't work when $2 is the path to a script, for example, "$HOME/.bash_script/alias_creator.sh" – Francisco Gallego Salido Apr 14 '15 at 11:23
  • 4
    Well.. 1. `[ -f $2 ]` - do you expect `$2` to be a path to some file? 2. `sudo tee -a`? You don't need `sudo` to write to your own home folder unless your permissions are messed up. 3. You could replace that big if condition with: `if ! grep -q "alias $1=" $HOME/.bash_aliases"; then`. – muru Apr 14 '15 at 11:24
  • 1
    @FranciscoGallegoSalido what happens in that case? An alias doesn't get created, or an incorrect one gets created? – muru Apr 14 '15 at 11:25
  • If the conditions aren't true, it just return an error message "Comando incorrecto" which means wrong command. $2 is meant to be either a command line or a path to a file. The sudo tee -a is just because, like you said, permissions are messed up. – Francisco Gallego Salido Apr 14 '15 at 11:29
  • Your script is very dangerous. Looks like I might destroy your system with ` rm "rm -rf /"`. – A.B. Apr 14 '15 at 12:01
  • Well, maybe in a future i'll look that ^^' – Francisco Gallego Salido Apr 14 '15 at 12:03
  • 1
    [Please translate the Spanish bits.](http://blog.stackoverflow.com/2009/07/non-english-question-policy/) It will make your question much more likely to get a reasonable answer (since it aids understanding of what the problem is), and will make the question and answer more usable for future users. – l0b0 Apr 14 '15 at 13:14
  • @A.B. there isn't any risk of erasing all files with tht command because it lacks the `--no-preserve-root` option. – 0x2b3bfa0 Apr 14 '15 at 13:48
  • @l0b0: In my answer the comments are translated (in fact I'm Spaish). – 0x2b3bfa0 Apr 14 '15 at 13:53
  • @FranciscoGallegoSalido: Please fix the permission of your `.bash_aliases` file with these commands: `sudo chown $USER:$USER ~/.bash_aliases` and `chmod go=,u=rwx .bash_aliases` – 0x2b3bfa0 Apr 14 '15 at 14:36
  • @FranciscoGallegoSalido: Please check [my answer](http://askubuntu.com/a/609136/387382) – 0x2b3bfa0 Apr 16 '15 at 10:43
  • Related (but not a duplicate, since that question is more general and this one is more about the details of writing a script): [How to easily make an alias command permanent?](https://askubuntu.com/questions/506995/how-to-easily-make-an-alias-command-permanent) – Eliah Kagan Apr 18 '15 at 15:31

3 Answers3

2

Here you're a corrected and improved script:

#!/bin/bash

###################################################
## Script to create permanent and unique aliases ##
###################################################

## alias is equal to the first argument.
alias="$1"

## command is equal to an array holding all
## the positional parameters minus the first.
## (i.e. for "ls -la" it would be ["ls, "-la"]).
command=("${@:2}")

## If .bash_aliases exists, load it. Now the alias check
## will take in account any alias not sourced yet on the current shell.
test -f ~/.bash_aliases && source $_

## If there are less than 2 parameters, prints the usage and exits.
if [ $# -lt 2 ]; then
    echo "Usage: $0 <alias_name> <command>"
    exit 1
fi

## If the command is not valid, prints an error and exits.
if ! type "${command[0]}" &> /dev/null; then
    echo "Wrong command"
    exit 3
fi

## If the current alias already exists, prints a error message and then exits.
if [[ "$(type -t '$alias')" == "alias" ]]; then
    echo "This alias alredy exists"
    exit 2
fi

## Append the alias to the .bash_aliases file.
echo "alias $alias='${command[@]}'" >> ~/.bash_aliases

## Print a sucess message.
echo "Alias '$alias' linked to the command '${command[@]}'!"

Things that I changed:

  • Commented code.
  • Used type to detemine if the alias already exists.
  • Removed the function because I think that is not necessary.
  • Used type to detemine if a command is valid or not.
  • Removed some unnecessary elif.
  • Improved the conditionals.
  • command may be unquoted (i.e. myscript pingoogle ping google.com)
0x2b3bfa0
  • 8,620
  • 5
  • 38
  • 59
2

Here are your errors (source):

  • SC2002 Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.
  • SC2006 Use $(..) instead of legacy `..`.
  • SC2046 Quote this to prevent word splitting.
  • SC2086 Double quote to prevent globbing and word splitting.
  • SC2126 Consider using grep -c instead of grep|wc.
   1  #!/bin/bash
   2  #Script to create unique and permanent aliases
   3  
   4  _add_alias()
   5  {
   6      echo "alias $1='$2'" | sudo tee -a $HOME/.bash_aliases
                                             ^––SC2086 Double quote to prevent globbing and word splitting.
   7      echo "Alias successfully created"
   8  }
   9  
  10  if [ `cat $HOME/.bash_aliases | grep "alias $1=" | wc -l` == 0 ]
           ^––SC2046 Quote this to prevent word splitting.
           ^––SC2006 Use $(..) instead of legacy `..`.
                ^––SC2086 Double quote to prevent globbing and word splitting.
                ^––SC2002 Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.
                                      ^––SC2126 Consider using grep -c instead of grep|wc.
  11      then
  12      if [ $# -eq 2 ]
  13          then
  14          if [ -f "$2" ] #This condition is always false, don't know why.
  15              then
  16              _add_alias $1 "$2"
                             ^––SC2086 Double quote to prevent globbing and word splitting.
  17          elif $2 > tmp.txt 2> tmp.txt
  18              then
  19              _add_alias $1 "$2"
                             ^––SC2086 Double quote to prevent globbing and word splitting.
  20          else
  21              echo "Wrong command"
  22          fi
  23          rm tmp.txt
  24      elif [ $# -gt 2 ]
  25          then
  26      echo "Error: Usage: addal alias_name 'command' (comand between quotes)"
  27      elif [ $# -lt 2 ]
  28          then
  29          echo "Wrong number of parameters"
  30      fi
  31  else
  32      echo "Alias name already exists"
  33  fi
A.B.
  • 89,123
  • 21
  • 245
  • 323
0

Some thoughts: 

myscript alias1 "$HOME/dir/ascript"

will evaluate in test part to

[ -f '/home/me/dir/ascript' ]

which may be true or false.

myscript alias2 '$HOME/dir/anotherscript'

will evaluate in test part to

[ -f '$HOME/dir/ascript' ]

which is always false (unless you have a dir named $HOME in local dir).

and 

myscript alias3 "$HOME/dir/sample.sh foo bar"

will evaluate in test part to

[ -f '/home/me/dir/sample.sh foo bar' ]

which is false, unless there is a 'sample.sh foo bar' file in /home/me/dir.

and you should fix .bash_aliases and use 

echo "alias $1='$2'" >> $HOME/.bash_aliases
muru
  • 193,181
  • 53
  • 473
  • 722
Archemar
  • 662
  • 8
  • 21