Re: GNUstep Printing Backend Patch and such

[Fred Kiefer]

Chad Hardin wrote:
> I'm done with the first part of turning GNUstep printing parts into a 
> "backend" type system.
>
> Basically, what happens now is that every-time an allocWithZone: is done 
> on a NSPageLayout, NSPrinter, NSPrintInfo, GSPrintOperation (not a type) 
> , or NSPrintPanel,  that class instead calls a new class, GSPrinting, 
> which loads up a Bundle located in Bundles/GSPrinting, and uses that 
> Bundle's principle class to get the real Class (GSLPRPrinter, for 
> example), allocates that and return it instead.
>
> My tests have shown that printing works the exact same as before, no 
> better or worse, which was my goal.  The only bundle that exists right 
> now is GSLPR, which is basically all the code that used to exist in the 
> NSPrint* and NSPageLayout classes.  Next will come the GSCUPS bundle , 
> which will be a real stress test and will probably result in a bit more 
> tweaking of both the NSPrint* and GSLPR* classes.
>
> Anyhow, here is the patch and the additional files I added for 
> everything to work.  It was suggested I share this with everyone before 
> doing anything with cvs.  :-)
> The Printing.tar.gz contains all the Printing backend budles, it 
> contains the Printing directory and goes in the gui directory.

I just started to look at the code of this great change and there are a 
few minor things that should be corrected before the final version:

- My opinion is that the new printing bundles should not be situated 
inside of GUI, but in some directory alongside to it, taking that huge 
amount of PPD files with them.

- You removed all the ivars and some of the enumerators in the header 
files. The ivars are fine, but even when GNUstep stops using the 
enumerator we need to supply them, as some OpenStep application may 
still rely on them and we don't want to frustrate a user. No big deal, 
as enums don't directly result in any code. You did copy these enums to 
the headers of the implementation files so there really was no gain.

- You added a method declaration -allocWithZone: to most of the classes. 
This should never be needed as this method is inherited from NSObject. 
Doing so to place an implementation detail comment doesn't seem to be a 
justifiaction for this extra declaration. Just remove it again.

- Was it needed to reformat all the method declarations and headers? 
They looked fine to me. Over all you seem to use a diffrent code 
formating than the rest of GNUstep, which is a bit annoying, when you 
just change the format of some methods. (Imagine me scrolling through 
your pathc file, finding all real changes as opposed to reformatting)

- Why did you add a -init method to NSPrintInfo, -initWithDictionary is 
the designated initializer, so this should do and be able to deal with a 
nil parameter. And why don't you use ASSIGN in the code for 
-setSharedPrintInfo: and replaced the RELEASE() in the -dealloc method?

- What is the use of spliting up the file NSPrintOperation.m into 
separate ones? It may be nice to have the one-class-per-file pattern, 
but the classes here are to simple to request separate files.

- When loading the GORM file for the panel you did stick with the old 
code, which even stated that it is wrong. Have a look at NSDataLinkPanel 
for a better way to do this.

- I also would expect that there is some more common code between the 
printing backends. You made some methods on the NS classes abstract that 
I can only think of one possible implementation. Either you have a lot 
more imagination than I, or you should bring back the implementation 
here to share it. Perhaps you need to implement a second backend first, 
to be sure which methods get shared.


After so many negative sounding statements I have to say that I fully 
support this split into printing backends and that you should go ahead 
with this.

Fred