Was this shell script ready for open-source? (Random XKCD wallpaper)

I wrote the following little shell script and published it on GitHub. In the repo there's also a README file that explains how to use it in more detail.

This is the first time I open-sourced/published something I wrote, so I'm interested whether you think that this code is ready for the public.

Some pointers, but feel free to add your own thoughts:

• Does it look professional? If not, what would need to be done?

• Are the comments too much? Was it ok to publish it, although I know that under some circumstances it doesn't work (I wrote about that in the README)?

```
# random xkcd wallpaper: gets a random xkcd comic from xkcd.com and sets it as the desktop background.

#!/bin/bash

# /random/comic redirects to random comic. Wget gets the index.html of the comic.
wget http://dynamic.xkcd.com/random/comic/

echo $(pwd)

#Searches the line in the index.html file that points to the url where the actual comic is placed.
#The image urls are of the form: http://imgs.xkcd.com/comics/.'name of comic'.(png | jpg)
url=$(cat index.html | grep -o -m 1 http://imgs.xkcd.com/comics/.*\.png)

#Assuming picture format is .png. Gets the name of the image file by only matching what comes after the last forward slash.
name_pic=$(echo $url | grep -o [^/]*\.png)
is_png=1

#Sets url and name_pic in the case of the picture being in .jpg format.
if [ -z "$url" ]
then
url=$(cat index.html | grep -o -m 1 http://imgs.xkcd.com/comics/.*\.jpg)
name_pic=$(echo $url | grep -o [^/]*\.jpg)
is_png=0
fi

#Downloads the image and saves it under its appropriate name.
wget --output-document="$name_pic" "$url"

#Sets the desktop background
gconftool-2 --set --type=string /desktop/gnome/background/picture_filename $(pwd)/"$name_pic"

#Cleans up
rm index.html

#For some reason, if the image is moved to fast (e.g without a wait) the background does not get set.
sleep 1

#The current wallpaper can always be found under "current_xkcd_wallpaper.png" or "current_xkcd_wallpaper.jpg". Al

#!/bin/bash

The shebang line has no effect unless it's on the very first line, so you should put it before the copyright notice.

echo $(pwd)

echo $(command) is almost always the same as just writing command (the exception being that echo adds a newline at the end if the command doesn't already produce one, but that's not the case here). So you can just change the above to pwd.

cat index.html | grep -o -m 1 http://imgs.xkcd.com/comics/.*\.png

That's useless use of cat. If you want to grep through a file, you can just pass the filename as the last argument - no need to cat and pipe. (The same goes for later places in the code where you do the same with .jpg).

gconftool-2 --set --type=string /desktop/gnome/background/picture_filename $(pwd)/"$name_pic"

I think it'd be a good idea to make the command to change the wallpaper configurable, so non-GNOME users can use your script too.

Also I'm not sure why you set the image to be the wallpaper before renaming it. I think it'd make more sense to do so afterwards (that might also solve your issue of having to sleep).

On a general note you might take care to gracefully handle already existing files. For example if the script is called in a directory where an index.html already exists (which is not that unlikely), your script will a) not work and b) delete that index.html, which its owner might not appreciate.

StackExchange Code Review Q#428, answer score: 15