Comment 3 for bug 249706

Revision history for this message
Colin Watson (cjwatson) wrote :

As I said in the Debian BTS, perhaps ssh itself should do this, rather than ssh-copy-id? You apparently ignored this comment. That said, that seems to be fiddly and maybe isn't worth it.

Your patch introduces 'set -ex'. -e doesn't make any difference in this code, and -x is noisy.

Your patch doesn't follow the indentation convention in that file. You should always follow the prevailing convention.

It would be better just to strip colons at the end, rather than throughout (which could produce very confusing results!). For that, just replacing $1 with ${1%:} would be entirely sufficient; you don't need to introduce another shell variable. (And why export HOSTNAME? Nothing outside this script needs it.)

In future, consider writing this instead of introducing an unnecessary call to the '[' program:

-if [ `echo "$1" |grep -c \: -` ]; then
+if echo "$1" | grep -q :; then

In the end, I've applied the attached patch to my Debian CVS repository. Thanks for nudging this along.