Index: /pykota/trunk/pykota/storages/sql.py
===================================================================
--- /pykota/trunk/pykota/storages/sql.py (revision 2765)
+++ /pykota/trunk/pykota/storages/sql.py (revision 2768)
@@ -527,8 +527,21 @@
         return groupsandquotas
         
-    def addPrinter(self, printername) :        
-        """Adds a printer to the quota storage, returns it."""
-        self.doModify("INSERT INTO printers (printername) VALUES (%s)" % self.doQuote(self.userCharsetToDatabase(printername)))
-        return self.getPrinter(printername)
+    def addPrinter(self, printer) :        
+        """Adds a printer to the quota storage, returns the old value if it already exists."""
+        try :
+            self.doModify("INSERT INTO printers (printername, passthrough, maxjobsize, description, priceperpage, priceperjob) VALUES (%s, %s, %s, %s, %s, %s)" \
+                              % (self.doQuote(self.userCharsetToDatabase(printer.Name)), \
+                                 self.doQuote((printer.PassThrough and "t") or "f"), \
+                                 self.doQuote(printer.MaxJobSize or 0), \
+                                 self.doQuote(self.userCharsetToDatabase(printer.Description)), \
+                                 self.doQuote(printer.PricePerPage or 0.0), \
+                                 self.doQuote(printer.PricePerJob or 0.0)))
+        except PyKotaStorageError :    
+            # TODO : check if this is an error different from a duplicate insert
+            # return the existing entry which has to be modified
+            return self.getPrinter(printer.Name)
+        else :    
+            printer.isDirty = False
+            return None # the entry created doesn't need further modification
         
     def addBillingCode(self, bcode) :
@@ -626,8 +639,8 @@
         self.doModify("UPDATE printers SET passthrough=%s, maxjobsize=%s, description=%s, priceperpage=%s, priceperjob=%s WHERE id=%s" \
                               % (self.doQuote((printer.PassThrough and "t") or "f"), \
-                                 self.doQuote(printer.MaxJobSize), \
+                                 self.doQuote(printer.MaxJobSize or 0), \
                                  self.doQuote(self.userCharsetToDatabase(printer.Description)), \
-                                 self.doQuote(printer.PricePerPage), \
-                                 self.doQuote(printer.PricePerJob), \
+                                 self.doQuote(printer.PricePerPage or 0.0), \
+                                 self.doQuote(printer.PricePerJob or 0.0), \
                                  self.doQuote(printer.ident)))
                                  
@@ -852,5 +865,5 @@
         printerids = ", ".join(["%s" % self.doQuote(p.ident) for p in printers])
         self.deleteInTransaction([ 
-                    "DELETE FROM printergroupsmembers WHERE groupid IN (%s) OR printerid IN (%s)" % printerids,
+                    "DELETE FROM printergroupsmembers WHERE groupid IN (%s) OR printerid IN (%s)" % (printerids, printerids),
                     "DELETE FROM jobhistory WHERE printerid IN (%s)" % printerids,
                     "DELETE FROM grouppquota WHERE printerid IN (%s)" % printerids,
Index: /pykota/trunk/pykota/storages/ldapstorage.py
===================================================================
--- /pykota/trunk/pykota/storages/ldapstorage.py (revision 2765)
+++ /pykota/trunk/pykota/storages/ldapstorage.py (revision 2768)
@@ -887,19 +887,24 @@
         return groupsandquotas
         
-    def addPrinter(self, printername) :        
-        """Adds a printer to the quota storage, returns it."""
-        printername = self.userCharsetToDatabase(printername)
+    def addPrinter(self, printer) :
+        """Adds a printer to the quota storage, returns the old value if it already exists."""
+        oldentry = self.getPrinter(printer.Name)
+        if oldentry.Exists :
+            return oldentry # we return the existing entry
+        printername = self.userCharsetToDatabase(printer.Name)
         fields = { self.info["printerrdn"] : printername,
                    "objectClass" : ["pykotaObject", "pykotaPrinter"],
                    "cn" : printername,
                    "pykotaPrinterName" : printername,
-                   "pykotaPricePerPage" : "0.0",
-                   "pykotaPricePerJob" : "0.0",
-                   "pykotaMaxJobSize" : "0",
-                   "pykotaPassThrough" : "0",
+                   "pykotaPassThrough" : (printer.PassThrough and "t") or "f",
+                   "pykotaMaxJobSize" : str(printer.MaxJobSize or 0),
+                   "description" : self.userCharsetToDatabase(printer.Description or ""),
+                   "pykotaPricePerPage" : str(printer.PricePerPage or 0.0),
+                   "pykotaPricePerJob" : str(printer.PricePerJob or 0.0),
                  } 
         dn = "%s=%s,%s" % (self.info["printerrdn"], printername, self.info["printerbase"])
         self.doAdd(dn, fields)
-        return self.getPrinter(printername)
+        printer.isDirty = False
+        return None # the entry created doesn't need further modification
         
     def addUser(self, user) :        
@@ -1094,8 +1099,8 @@
         fields = {
                    "pykotaPassThrough" : (printer.PassThrough and "t") or "f",
-                   "pykotaMaxJobSize" : (printer.MaxJobSize and str(printer.MaxJobSize)) or "0",
+                   "pykotaMaxJobSize" : str(printer.MaxJobSize or 0),
                    "description" : self.userCharsetToDatabase(printer.Description or ""),
-                   "pykotaPricePerPage" : str(printer.PricePerPage),
-                   "pykotaPricePerJob" : str(printer.PricePerJob),
+                   "pykotaPricePerPage" : str(printer.PricePerPage or 0.0),
+                   "pykotaPricePerJob" : str(printer.PricePerJob or 0.0),
                  }
         self.doModify(printer.ident, fields)
Index: /pykota/trunk/bin/pkbcodes
===================================================================
--- /pykota/trunk/bin/pkbcodes (revision 2765)
+++ /pykota/trunk/bin/pkbcodes (revision 2768)
@@ -106,42 +106,41 @@
                 raise PyKotaCommandLineError, _("There's no billingcode matching %s") % " ".join(names)
                         
-        reset = options["reset"]
-        description = options["description"]
-        if description :
-            description = options["description"].strip()
-                
-        if options["delete"] :    
+        if options["list"] :
+            for billingcode in billingcodes :
+                print "%s [%s] %s %s %s %.2f %s" % \
+                      (billingcode.BillingCode, billingcode.Description, \
+                       billingcode.PageCounter, \
+                       _("pages"), \
+                       _("and"), \
+                       billingcode.Balance, \
+                       _("credits"))
+        elif options["delete"] :    
             self.display("\n%s..." % _("Deletion"))
             self.storage.deleteManyBillingCodes(billingcodes)
             self.display("\n")
-        elif options["add"] :    
-            self.display("%s...\n" % _("Creation"))
-            nbtotal = len(names)
-            for i in range(nbtotal) :
-                bname = names[i]
-                billingcode = StorageBillingCode(self.storage, bname)
-                self.modifyBillingCode(billingcode, reset, description)
-                oldbillingcode = self.storage.addBillingCode(billingcode)
-                if oldbillingcode is not None :
-                    if options["skipexisting"] :
-                        self.printInfo(_("Billing code [%s] already exists, skipping.") % bname)
-                    else :    
-                        self.printInfo(_("Billing code [%s] already exists, will be modified.") % bname)
-                        self.modifyBillingCode(oldbillingcode, reset, description)
-                        oldbillingcode.save()
-                percent = 100.0 * float(i) / float(nbtotal)
-                self.display("\r%.02f%%" % percent)
-        else :        
-            if options["list"] :
-                for billingcode in billingcodes :
-                    if billingcode.Exists :
-                        print "%s [%s] %s %s %s %.2f %s" % \
-                          (billingcode.BillingCode, billingcode.Description, \
-                           billingcode.PageCounter, \
-                           _("pages"), \
-                           _("and"), \
-                           billingcode.Balance, \
-                           _("credits"))
-            else :               
+        else :
+            reset = options["reset"]
+            description = options["description"]
+            if description :
+                description = options["description"].strip()
+            skipexisting = options["skipexisting"]        
+            if options["add"] :    
+                self.display("%s...\n" % _("Creation"))
+                nbtotal = len(names)
+                for i in range(nbtotal) :
+                    bname = names[i]
+                    billingcode = StorageBillingCode(self.storage, bname)
+                    self.modifyBillingCode(billingcode, reset, description)
+                    oldbillingcode = self.storage.addBillingCode(billingcode)
+                    if oldbillingcode is not None :
+                        if skipexisting :
+                            self.printInfo(_("Billing code [%s] already exists, skipping.") % bname)
+                        else :    
+                            self.printInfo(_("Billing code [%s] already exists, will be modified.") % bname)
+                            self.modifyBillingCode(oldbillingcode, reset, description)
+                            oldbillingcode.save()
+                    percent = 100.0 * float(i) / float(nbtotal)
+                    self.display("\r%.02f%%" % percent)
+            else :        
                 self.display("\n%s...\n" % _("Modification"))
                 nbtotal = len(billingcodes)            
@@ -152,5 +151,5 @@
                     percent = 100.0 * float(i) / float(nbtotal)
                     self.display("\r%.02f%%" % percent)
-                    
+                        
         if not options["list"] :                
             self.done()
Index: /pykota/trunk/bin/pkprinters
===================================================================
--- /pykota/trunk/bin/pkprinters (revision 2725)
+++ /pykota/trunk/bin/pkprinters (revision 2768)
@@ -30,4 +30,5 @@
 
 from pykota.tool import PyKotaTool, PyKotaToolError, PyKotaCommandLineError, crashed, N_
+from pykota.storage import StoragePrinter
 
 __doc__ = N_("""pkprinters v%(__version__)s (c) %(__years__)s %(__author__)s
@@ -124,4 +125,24 @@
 class PKPrinters(PyKotaTool) :        
     """A class for a printers manager."""
+    def modifyPrinter(self, printer, charges, perpage, perjob, description, passthrough, nopassthrough, maxjobsize) :
+        if charges :
+            printer.setPrices(perpage, perjob)    
+        if description is not None :        # NB : "" is allowed !
+            printer.setDescription(description)
+        if nopassthrough :    
+            printer.setPassThrough(False)
+        if passthrough :    
+            printer.setPassThrough(True)
+        if maxjobsize is not None :
+            printer.setMaxJobSize(maxjobsize)
+            
+    def managePrintersGroups(self, pgroups, printer, remove) :        
+        """Manage printer group membership."""
+        for pgroup in pgroups :
+            if remove :
+                pgroup.delPrinterFromGroup(printer)
+            else :
+                pgroup.addPrinterToGroup(printer)    
+                
     def main(self, names, options) :
         """Manage printers."""
@@ -129,121 +150,113 @@
             raise PyKotaCommandLineError, "%s : %s" % (pwd.getpwuid(os.geteuid())[0], _("You're not allowed to use this command."))
             
-        if options["delete"] :    
-            self.display("%s...\n" % _("Deletion"))
-            todelete = self.storage.getMatchingPrinters(",".join(names))
-            nbtotal = len(todelete)
-            for i in range(nbtotal) :
-                entry = todelete[i]
-                if entry.Exists :
-                    entry.delete()
-                percent = 100.0 * float(i) / float(nbtotal)
-                self.display("\r%.02f%%" % percent)
+        if not options["add"] :
+            if not options["list"] :
+                self.display(_("Extracting datas..."))
+            if not names :      # NB : can't happen for --delete because it's catched earlier
+                names = ["*"]
+            printers = self.storage.getMatchingPrinters(",".join(names))
+            if not printers :
+                raise PyKotaCommandLineError, _("There's no printer matching %s") % " ".join(names)
+                
+        if options["list"] :
+            for printer in printers :
+                parents = ", ".join([p.Name for p in self.storage.getParentPrinters(printer)])
+                print "%s [%s] (%s + #*%s)" % \
+                      (printer.Name, printer.Description, printer.PricePerJob, \
+                       printer.PricePerPage)
+                print "    %s" % (_("Passthrough mode : %s") % ((printer.PassThrough and _("ON")) or _("OFF")))
+                print "    %s" % (_("Maximum job size : %s") % ((printer.MaxJobSize and (_("%s pages") % printer.MaxJobSize)) or _("Unlimited")))
+                if parents : 
+                    print "    %s %s" % (_("in"), parents)
+                print    
+        elif options["delete"] :    
+            self.display("\n%s..." % _("Deletion"))
+            self.storage.deleteManyPrinters(printers)
+            self.display("\n")
         else :
+            if options["groups"] :        
+                printersgroups = self.storage.getMatchingPrinters(options["groups"])
+                if not printersgroups :
+                    raise PyKotaCommandLineError, _("There's no printer matching %s") % " ".join(options["groups"].split(','))
+            else :         
+                printersgroups = []
+                    
+            if options["charge"] :
+                try :
+                    charges = [float(part) for part in options["charge"].split(',', 1)]
+                except ValueError :    
+                    raise PyKotaCommandLineError, _("Invalid charge amount value %s") % options["charge"]
+                else :    
+                    if len(charges) > 2 :
+                        charges = charges[:2]
+                    if len(charges) != 2 :
+                        charges = [charges[0], None]
+                    (perpage, perjob) = charges
+            else :        
+                charges = perpage = perjob = None
+                    
+            if options["maxjobsize"] :        
+                try :
+                    maxjobsize = int(options["maxjobsize"])
+                    if maxjobsize < 0 :
+                        raise ValueError
+                except ValueError :    
+                    raise PyKotaCommandLineError, _("Invalid maximum job size value %s") % options["maxjobsize"]
+            else :        
+                maxjobsize = None
+                    
+            description = options["description"]
+            if description :
+                description = description.strip()
+                
+            nopassthrough = options["nopassthrough"]    
+            passthrough = options["passthrough"]
+            remove = options["remove"]
+            skipexisting = options["skipexisting"]
             if options["add"] :    
                 self.display("%s...\n" % _("Creation"))
-                printers = []
                 nbtotal = len(names)
                 for i in range(nbtotal) :
                     pname = names[i]
-                    printer = self.storage.getPrinter(pname)
-                    if printer.Exists :
-                        if options["skipexisting"] :
-                            self.printInfo(_("Printer %s already exists, skipping.") % printer.Name)
-                        else :    
-                            self.printInfo(_("Printer %s already exists, will be modified.") % printer.Name)
-                            printers.append(printer)
-                    else :
-                        if self.isValidName(pname) :
-                            printer = self.storage.addPrinter(pname)
-                            if not printer.Exists :
-                                raise PyKotaToolError, _("Impossible to add printer %s") % pname
+                    if self.isValidName(pname) :
+                        printer = StoragePrinter(self.storage, pname)
+                        self.modifyPrinter(printer, charges, perpage, perjob,\
+                                       description, passthrough, \
+                                       nopassthrough, maxjobsize)
+                        oldprinter = self.storage.addPrinter(printer)               
+                        if oldprinter is not None :
+                            if skipexisting :
+                                self.printInfo(_("Printer %s already exists, skipping.") % pname)
                             else :    
-                                printers.append(printer)
-                        else :    
-                            raise PyKotaCommandLineError, _("Invalid printer name %s") % pname
+                                self.printInfo(_("Printer %s already exists, will be modified.") % pname)
+                                self.modifyPrinter(oldprinter, charges, \
+                                           perpage, perjob, description, \
+                                           passthrough, nopassthrough, \
+                                           maxjobsize)
+                                oldprinter.save()           
+                                self.managePrintersGroups(printersgroups, oldprinter, remove)
+                        elif printersgroups :        
+                            self.managePrintersGroups(printersgroups, \
+                                                      self.storage.getPrinter(pname), \
+                                                      remove)
+                    else :    
+                        raise PyKotaCommandLineError, _("Invalid printer name %s") % pname
                     percent = 100.0 * float(i) / float(nbtotal)
                     self.display("\r%.02f%%" % percent)
-                self.display("\r100.00%%\r        \r%s\n" % _("Done."))
             else :        
-                if not names :
-                    names = ["*"]
-                printers = self.storage.getMatchingPrinters(",".join(names))
-                if not printers :
-                    raise PyKotaCommandLineError, _("There's no printer matching %s") % " ".join(names)
-                        
-            if options["list"] :
-                for printer in printers :
-                    if printer.Exists :
-                        parents = ", ".join([p.Name for p in self.storage.getParentPrinters(printer)])
-                        print "%s [%s] (%s + #*%s)" % \
-                              (printer.Name, printer.Description, printer.PricePerJob, \
-                               printer.PricePerPage)
-                        print "    %s" % (_("Passthrough mode : %s") % ((printer.PassThrough and _("ON")) or _("OFF")))
-                        print "    %s" % (_("Maximum job size : %s") % ((printer.MaxJobSize and (_("%s pages") % printer.MaxJobSize)) or _("Unlimited")))
-                        if parents : 
-                            print "    %s %s" % (_("in"), parents)
-                        print    
-            else :
-                self.display("%s...\n" % _("Modification"))
-                
-                if options["groups"] :        
-                    printersgroups = self.storage.getMatchingPrinters(options["groups"])
-                    if not printersgroups :
-                        raise PyKotaCommandLineError, _("There's no printer matching %s") % " ".join(options["groups"].split(','))
-                else :         
-                    printersgroups = []
-                        
-                if options["charge"] :
-                    try :
-                        charges = [float(part) for part in options["charge"].split(',', 1)]
-                    except ValueError :    
-                        raise PyKotaCommandLineError, _("Invalid charge amount value %s") % options["charge"]
-                    else :    
-                        if len(charges) > 2 :
-                            charges = charges[:2]
-                        if len(charges) != 2 :
-                            charges = [charges[0], None]
-                        (perpage, perjob) = charges
-                        
-                if options["maxjobsize"] :        
-                    try :
-                        maxjobsize = int(options["maxjobsize"])
-                        if maxjobsize < 0 :
-                            raise ValueError
-                    except ValueError :    
-                        raise PyKotaCommandLineError, _("Invalid maximum job size value %s") % options["maxjobsize"]
-                else :        
-                    maxjobsize = None
-                        
-                description = options["description"]
-                if description :
-                    description = description.strip()
-                    
-                nopassthrough = options["nopassthrough"]    
-                passthrough = options["passthrough"]
+                self.display("\n%s...\n" % _("Modification"))
                 nbtotal = len(printers)
                 for i in range(nbtotal) :        
                     printer = printers[i]
-                    if options["charge"] :
-                        printer.setPrices(perpage, perjob)    
-                    if description is not None :        # NB : "" is allowed !
-                        printer.setDescription(description)
-                    if nopassthrough and printer.PassThrough :    
-                        printer.setPassThrough(False)
-                    if passthrough and not printer.PassThrough :    
-                        printer.setPassThrough(True)
-                    if (maxjobsize is not None) and (printer.MaxJobSize != maxjobsize) :    
-                        printer.setMaxJobSize(maxjobsize)
+                    self.modifyPrinter(printer, charges, perpage, perjob, \
+                                       description, passthrough, \
+                                       nopassthrough, maxjobsize)
                     printer.save()    
-                    for pgroup in printersgroups :
-                        if options["remove"] :
-                            pgroup.delPrinterFromGroup(printer)
-                        else :
-                            pgroup.addPrinterToGroup(printer)    
+                    self.managePrintersGroups(printersgroups, printer, remove)
                     percent = 100.0 * float(i) / float(nbtotal)
                     self.display("\r%.02f%%" % percent)
                                 
         if not options["list"] :                
-            self.display("\r100.00%%\r        \r%s\n" % _("Done."))
+            self.done()
                      
 if __name__ == "__main__" : 
@@ -285,9 +298,10 @@
            or (options["skipexisting"] and not options["add"]) \
            or (options["list"] and (options["add"] or options["delete"] or options["groups"] or options["charge"] or options["remove"] or options["description"])) \
-           or (options["passthrough"] and options["nopassthrough"]) :
+           or (options["passthrough"] and options["nopassthrough"]) \
+           or (options["remove"] and options["add"]) :
             raise PyKotaCommandLineError, _("incompatible options, see help.")
-        elif options["remove"] and not options["groups"] :    
+        elif options["remove"] and not options["groups"] :
             raise PyKotaCommandLineError, _("You have to pass printer groups names on the command line")
-        elif (not args) and (not options["list"]) :    
+        elif (not args) and (options["add"] or options["delete"]) :
             raise PyKotaCommandLineError, _("You have to pass printer names on the command line")
         else :
Index: /pykota/trunk/NEWS
===================================================================
--- /pykota/trunk/NEWS (revision 2767)
+++ /pykota/trunk/NEWS (revision 2768)
@@ -24,5 +24,5 @@
     - 1.24alpha15 (2006-03-01) :
         
-        - optimized pkbcodes using edpykota as the model.
+        - optimized pkbcodes and pkprinters using edpykota as the model.
         
         - 'edpykota --list' and 'pkusers --list' are now authorized 
