bug-gnustep
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: GNUstep Printing Backend Patch and such


From: Fred Kiefer
Subject: Re: GNUstep Printing Backend Patch and such
Date: Mon, 05 Jul 2004 12:33:10 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040114

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]