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". Also prevents cluttering the directory with images. 
#If you want to keep all the pictures you downloaded, uncomment the following lines.

#Cleans up. Makes sure that there is only one current_xkcd_wallpaper image file.
#rm current_xkcd_wallpaper.*

rm current_xkcd_wallpaper.*

if [ $is_png = 1 ] ; then
    mv $(pwd)/"$name_pic" $(pwd)/current_xkcd_wallpaper.png
else
    mv $(pwd)/"$name_pic" $(pwd)/current_xkcd_wallpaper.jpg
fi
#!/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.