Comment 4 for bug 249706

On Thu, October 9, 2008 21:25, Colin Watson wrote:
> As I said in the Debian BTS, perhaps ssh itself should do this, rather
> than ssh-copy-id? You apparently ignored this comment.

I have no C, which i assume is what ssh proper is written in.

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.

Sorry, I should have cleaned up after testing.

> 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

I didnt know this was posible, so thanks for letting me know. I'll try and
familiarise myself with it.

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

Thanks for fixing it up.
kk