Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006448 [Squeak] Morphic minor always 04-27-07 21:13 09-25-07 04:52
Reporter adxp View Status public  
Assigned To
Priority normal Resolution reopened  
Status feedback   Product Version 3.9
Summary 0006448: Pretty printer breaks: two newlines added after semicolon in a cascade
Description 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)]].
Additional Information
Attached Files  snapshot1.png [^] (65,629 bytes) 04-30-07 17:34
 snapshot2.png [^] (61,207 bytes) 04-30-07 17:34

- Relationships

- Notes
(0010618 - 31 - 31 - 31 - 139 - 139 - 139)
adxp
04-27-07 21:14

This is using version 3.9 0007068
 
(0010619 - 242 - 242 - 242 - 242 - 242 - 242)
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 - 553 - 655 - 655 - 655 - 655 - 655)
wiz
04-29-07 01:05
edited on: 04-29-07 01:07

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 - 268 - 268 - 268 - 268 - 268 - 268)
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 - 447 - 507 - 507 - 507 - 507 - 507)
wiz
05-01-07 03:26
edited on: 05-01-07 03:27

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 - 285 - 309 - 309 - 309 - 309 - 309)
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 - 466 - 514 - 514 - 514 - 514 - 514)
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 - 1269 - 1383 - 1383 - 1383 - 1383 - 1383)
wiz
09-23-07 00:08
edited on: 09-23-07 02:16

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 - 1859 - 2049 - 2049 - 2049 - 2049 - 2049)
nicolas cellier
09-24-07 21:16
edited on: 09-24-07 21:17

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 - 536 - 590 - 590 - 590 - 590 - 590)
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
 

- Issue History
Date Modified Username Field Change
04-27-07 21:13 adxp New Issue
04-27-07 21:14 adxp Note Added: 0010618
04-27-07 21:16 KenCausey Note Added: 0010619
04-29-07 01:05 wiz Note Added: 0010624
04-29-07 01:06 wiz Status new => resolved
04-29-07 01:06 wiz Resolution open => no change required
04-29-07 01:06 wiz Fixed in Version  => 3.9
04-29-07 01:07 wiz Note Edited: 0010624
04-30-07 17:34 KenCausey File Added: snapshot1.png
04-30-07 17:34 KenCausey File Added: snapshot2.png
04-30-07 17:36 KenCausey Note Added: 0010635
05-01-07 03:26 wiz Note Added: 0010641
05-01-07 03:26 wiz Status resolved => feedback
05-01-07 03:26 wiz Resolution no change required => reopened
05-01-07 03:26 wiz Fixed in Version 3.9 =>
05-01-07 03:27 wiz Note Edited: 0010641
09-20-07 01:53 wiz Note Added: 0011157
09-20-07 21:48 KenCausey Note Added: 0011174
09-23-07 00:08 wiz Note Added: 0011178
09-23-07 02:16 wiz Note Edited: 0011178
09-24-07 21:16 nicolas cellier Note Added: 0011189
09-24-07 21:17 nicolas cellier Note Edited: 0011189
09-25-07 04:52 wiz Note Added: 0011200


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
93 total queries executed.
51 unique queries executed.
Powered by Mantis Bugtracker