and so i put my perl books aside.....

[Frederic Peters]

Oliver Grawert wrote:

> i would like to roll it out to the masses, but as it is my first python
> ride, i would like to have some enlighted snake goucho for a review,
> since i'm messing with a important system file here (i'm sure it works
> with any kind of ubuntu default menu.lst, but i have not tested it on
> any heavy tweaked version).
> 
> finally a link:
> http://www.grawert.net/software/startup-settings/

Great; patch to get it more pythonic (my taste) attached.


> feel free to critizise a lot, since i want to become a good pythoneer !

Some comments you could get from a Python programmer:


-               my_uid = apply(getattr(os, 'getuid'), ())
+               my_uid = os.getuid()

I gave away my Perl books long ago; not sure where this idiom may come
from.


-       for key in oslist[:]:
+       for i, title in enumerate(titles):

Hashes are great; both in Python and Perl.  But oslist was perfect for
the 'list' type.  enumerate() creates (index, item) couples:

>>> titles = ['Debian', 'Ubuntu']
>>> for i, title in enumerate(titles):
...     print title, 'has index', i
... 
Debian has index 0
Ubuntu has index 1


-                       p = re.compile('\n')
-                       line = p.sub('', line)
-                       if re.search('^timeout', line):
+                       line = line.strip()
+                       if line.startswith('timeout'):

Two things here: Python programmers don't use regular expressions for
everything, builtin string functions are more common.

And relevant to this particular program, grub lines may start with
spaces, strip() will strip them (as well as new lines or tabulations,
at the beginning and at the end).


> ps: python is really fast gui development (took me two evenings to write
> this without any knowledge of the lang), ... you see this perl geezer
> impressed ! 

Best advice would now be to get used to the standard library.



Regards,

        Frederic

-------------- next part --------------
--- startup_settings-0.1.orig/startup-settings	2004-11-15 00:14:52.000000000 +0100
+++ startup_settings-0.1/startup-settings	2004-11-15 15:12:45.000000000 +0100
@@ -17,7 +17,10 @@
 	self.window.set_type_hint('GDK_WINDOW_TYPE_HINT_DIALOG')
 	self.window.set_resizable(gtk.FALSE)
 	self.window.set_position('GTK_WIN_POS_CENTER_ALWAYS')
-	self.window.set_icon_from_file("/usr/share/startup-settings/startup-settings.png")
+	if os.path.exists("/usr/share/startup-settings/startup-settings.png"):
+		self.window.set_icon_from_file("/usr/share/startup-settings/startup-settings.png")
+	else:
+		self.window.set_icon_from_file("img/startup-settings.png")
 	
 	# initial layout elements
 	vbox = gtk.VBox(gtk.FALSE, 5)
@@ -49,9 +52,7 @@
 	hbox1.pack_start(label1, gtk.FALSE, gtk.FALSE, 10)
 
 	# get defaultdata from sub method
-	time,default,array = self.read_menulist()
-	time = int(time)
-	default = int(default)
+	time, default, titles = self.read_menulist()
 
 	# add a spinbutton
 	self.adj = gtk.Adjustment(time, 3, 60, 1, 5, 0)
@@ -81,18 +82,16 @@
 	hbox2.pack_start(self.combo, gtk.FALSE, gtk.TRUE, 0)
 
 	# fill list with os names (filter non necessary entrys)
-	oslist = array.keys()
-	a=0
-	for key in oslist[:]:
-		if not re.search('Memory test', array[key]) \
-		and not re.search('(recovery mode)', array[key]) \
-		and not re.search('Failsafe', array[key]):
-			iter = list.append((key , array[key]) )
+	a = 0
+	for i, title in enumerate(titles):
+		if not 'Memory test' in title and \
+				not '(recovery mode)' in title and \
+				not 'Failsafe' in title:
+			iter = list.append((i, title))
 			list.set(iter)
-			oslist.pop()
-			if default == key:
+			if default == i:
 				self.combo.set_active(a)
-			a = a+1
+			a += 1
 
 	# two buttons
 	helpbutton = gtk.Button("",gtk.STOCK_HELP);
@@ -123,9 +122,9 @@
 
 		# parse the file (replace timeout and default)
 		for readline in infile.readlines():
-			if re.search('^timeout', readline):
+			if readline.strip().startswith('timeout'):
 				readline = 'timeout\t\t'+str(sek)+'\n'
-			elif re.search('^default', readline):
+			elif readline.strip().startswith('default'):
 				readline = 'default\t\t'+str(data0)+'\n'
 			outfile.write(readline)
 		
@@ -143,32 +142,29 @@
     def read_menulist(self):
     		# recieve the defaults form menu.lst
     		file = open('/boot/grub/menu.lst','r')
-		array = {}
+		titles = []
+
+		time = '5'    # default
+		default = '0' # default
 
 		# read and filter the input
 		for line in file.readlines():
-			p = re.compile('\n')
-			line = p.sub('', line)
-			if re.search('^timeout', line):
-				p = re.compile('timeout\t\t')
-				line = p.sub('', line)
-				time = line
-			elif re.search('^default', line):
-				p = re.compile('default\t\t')
-				line = p.sub('', line)
-				default = line
-			elif re.search('^title', line):
-				if not re.search('Other operating systems:', line):
-					p = re.compile('title\t\t')
-					line = p.sub('', line)
-					array[len(array)] = line
+			line = line.strip()
+			if line.startswith('timeout'):
+				time = line[7:].strip()
+			elif line.startswith('default'):
+				default = line[7:].strip()
+			elif line.startswith('title'):
+				if not 'Other operating systems:' in line:
+					title = line[5:].strip()
+					titles.append(title)
 
 		# return the input to caller
-		return time,default,array
+		return int(time), int(default), titles
 
     def check_uid(self):
     		# check if we are root enabled
-    		my_uid = apply(getattr(os, 'getuid'), ())
+    		my_uid = os.getuid()
 		if my_uid != 0:
 			self.error()
 
@@ -184,7 +180,7 @@
 		# display a descriptive text
 		self.dialog.set_markup("<b>Access Error</b>\n   This tool requires administrative rights.\n   Call it with gksudo to get root privileges")
 		response = self.dialog.run()
-		sys.exit()
+		sys.exit(1)
 
     def yelp(self, widget):
     		# call the help viewer