Mantis - Squeak
Viewing Issue Advanced Details
6448 Morphic minor always 04-27-07 21:13 09-25-07 04:52
adxp  
 
normal  
feedback 3.9  
reopened  
none    
none  
0006448: Pretty printer breaks: two newlines added after semicolon in a cascade
Pretty printer adds extraneous blank lines when pretty printing the following:

renderContentOn: html
        html div id: 'container';
                with: [html div id: 'header';
                                with: [html div id: 'inside';
                                                with: [html div class: 'logo';
                                                                with: [html anchor
                                                                                callback: [self home];
                                                                                with: [html image url: '/images/logo.png']].
                                                           html div id: 'nav';
                                                                with: [html unorderedList id: 'main-nav';
                                                                                with: [self nav: html]].]]].
                                html div id: 'content';
                                        with: [html div id: 'content-inside';
                                                         with: [html div class: 'column'; with: (self content)]].
 snapshot1.png [^] (65,629 bytes) 04-30-07 17:34
 snapshot2.png [^] (61,207 bytes) 04-30-07 17:34

Notes
(0010618)
adxp   
04-27-07 21:14   
This is using version 3.9 0007068
(0010619)
KenCausey   
04-27-07 21:16   
I was working with adxp to verify this on #Squeak and was able to do so in 3.9-final-7067. A simpler method I created with a single line and a cascade did not do this. I suspect some sort of complicated interaction involving all the blocks.
(0010624)
wiz   
04-29-07 01:05   
You mean tabs rather than blank lines.

I thing you get a tab for both the [ and the ;

thus two tabs added for each line.

without the block things work as expected. From your example:

                                                                with: [html anchor
                                                                                callback: [self home];
                                                                                with: [html image url: '/images/logo.png']].


So I don't see this is a bug.

    

(0010635)
KenCausey   
04-30-07 17:36   
Please look at the two snapshots I have attached. In the first I have taken adxp's example and formatted it more to my tastes. In the second I have chosen the 'prettyPrint' view as you can see from the button at the far right. Are tabs going to produce this result?
(0010641)
wiz   
05-01-07 03:26   
Ok. You mean newlines or linefeeds.

So what causes the difference? And why are the lines
                                                                with: [html anchor
                                                                                callback: [self home];
                                                                                with: [html image url: '/images/logo.png']].


Not subject to the same problem?

(0011157)
wiz   
09-20-07 01:53   
Hi Ken,

Can you also upload a snap of the offending method. I looked at the pictures again and realized I couldn't tell whether the problem was too many tabs causing extra lines or just too many lines. (All white space looks alike.)

Yours in curiosity and service, --Jerome Peace
(0011174)
KenCausey   
09-20-07 21:48   
Jerome,

I don't understand your request. The method is visible in both snapshots.

Also I don't understand your whitespace comment. How can a tab and a carriage return be indistinguishable?

I entered this code in the usual fashion; the tab positions are what the editor gave me by default in most if not all cases. In other words I'm not sure I ever hit the tab key explicitly, and I hit the enter key at the end of each line.

What else can I tell you?
(0011178)
wiz   
09-23-07 00:08   
Hi Ken,

Thank you for your timely response.

Apologies for the confusion.

First I saw your note and requested help of you rather that the author of the report adxp which is who I should have directed things to.

Second the request for the code goodie to be up loaded was done hastily before I glanced again at the text snap shots. I left it because copying and pasting across platforms does strange things to line endings and tabs so the pasted text does not compile directly on my system.
Also the first rule of trouble shooting is get first hand information.

Third, (I did manage to massage the text to compile) There are things to notice about the white space on the blank lines. Each bank line actually contains the same amount of tabs as the following line. That is a clue that directs me to some suspects and eliminates others.

Sorry for the confusion. For the record, I would still like to see an upload of the source code of the problem. Or even better a cleaner, leaner example that produces the same errant behavior. I could and would do something like that myself, but if I am forced to do it on my own, it will be done with a lower priority than if help is being given.

Thanks again.

Yours in service and curiosity, --Jerome Peace

(0011189)
nicolas cellier   
09-24-07 21:16   
Diagnostic:
  MessageNode>>printKeywords: key arguments: args on: aStream indent: level
    prefix: isPrefix

do prefix the message with crtab: if complex message (take a block arg or a keyword message send as one of the arguments)

while its sender:
  CascadeNode>>printOn: aStream indent: level precedence: p

do postfix each complex cascade (binary or keyword message) with a crtab:
hence double crtab...

Please note that the isPrefix argument is not used (has a single caller set to true for printing optimized #to:do:). Since the message has a single implementor, this is a dead argument. Could it have any intention related with this problem? Browsing older implementation seems to indicate it's rather related to alternate syntax.
Due to current level of comments (none!) we now depend on good will of gurus who wrote this code (I remember i saw some pieces of this code in st80, so good luck).

One ugly solution would be to add an argument indicating whether we should crtab: or not (because we already did it in the cascade).

Seeing the code, i would much prefer a TreeIterator pattern rewrite (also known as visitor).
Why? a PrettyPrinter object can hold states like indent level, precedence logic, and newline status in inst vars. That is far easier than trying to pass the whole state in a bunch of arguments.
It would also be far easier to program individual preferences in such a class (or eventually a subclass) than duplicating all printOn: message hierarchy, or trying to add even more arguments to them for passing the preferences...

Visitor pattern is simple and efficient, i experimented it to introduce syntax highlighting in the mid 90s and to print mathematical expressions in whatever format (LaTeX, maple, Morph-like Graphical output etc...). That's also the solution adopted by VW now for formatting code.

(0011200)
wiz   
09-25-07 04:52   
Hi Nicolas,

Thanks for your diagnosis.

On my wish list is a way to have a pretty printer that can be controlled to do my bidding. Allowing one to write variant pretty prints to ones hearts content.
I have noticed that currently this would mean multi methods in each node. So I agree something more general based on a visitor is called for.

I am wondering if shout does something special ( though I suspect it just adjusts colors and lets the white space fall where it may.)

Yours in curiosity and service, --Jerome Peace