Hi guys

here is some feedback from John. I share it with you.

On 02 Sep 2015, at 16:05, John Brant <brant@refactoryworkers.com> wrote:

I've been writing some automated scripts to port SmaCC to Pharo. I have it where I can file in the Dolphin .pac files directly into a Pharo 5 image. Most things are working, but I've had to fight some Pharo issues. Since I wasn't sure who should be notified, I thought I'd complain to you :)

:) if you would see my list of complaints about Pharo you would laugh a lot because it is huge and diverse.
I put thierry on CC because he is the guy maintaining and using SmaCC a lot.
So may be you can help each others.

*) The BIConfigurableFormatter (aka RBConfigurableFormatter) sends #asString to nodes. First, it should format the node if it wants them formatted instead of relying on asString to return a formatted string. Second, by sending #asString, formatting has become exponential in the nesting level of blocks. For example, Dolphin Smalltalk runs this in 4 milliseconds:

�� | stream |
�� stream := WriteStream on: String new.
�� stream nextPutAll: 'foo ^true and: '.
�� 40 timesRepeat: [stream nextPutAll: '[true and: '].
�� stream nextPutAll: 'false'.
�� 40 timesRepeat: [stream nextPut: $]].
�� (RBParser parseMethod: stream contents) formattedCode

I'm guessing that Pharo wouldn't finish in a day... It takes over 4 seconds to format 16 nested blocks.

Thanks indeed we will fix it.
https://pharo.fogbugz.com/f/cases/16462/BIConfigurableFormatter-should-not-use-asString

*) Also on the formatter, its default doesn't appear to be compliant with Kent's blocks should look like blocks/rectangles/box rule. The way I interpret the rule is that if I draw a rectangle around everything in the block including the brackets, it should not include any text of anything outside of the block. For example, this would fail that test:

surroundedBy: aString
�� ^ self class
�� streamContents: [ :s |
�� s nextPutAll: aString.
�� s nextPutAll: self.
�� s nextPutAll: aString ]

�� yes

Since the block starts at the end of the #streamContents: line, a rectangle around all the contents in the block contains part of the #streamContents: selector which isn't part of the block.

I do not really like all the formattings of kent. Because sometimes it just does not work.
But this is why we ask franck (an interns to start to have a look).

For example I do not like

�� self xxx
�� ifTrue:
�� [
�� self bla
�� self foo.

in the current default formatter.
�� and I prefer
��
�� self xxx
�� ifTrue: [ self bla.
�� self foo
But here the problem is that the�� self foo�� is not tab aligned because ifTrue: [

�� self xxx
�� ifTrue:
�� [ self bla.
�� self foo

So may be this one is a nice compromise
I think that we should change some settings. In fact in alpha things are there to evolve until the release :)

*) If I fileout a package, it doesn't fileout the metaclass extension methods. The RPackage>>fileOut method needs to be updated with the metaclassExtensionSelectors.

Oops this is a bug.
Do you mean what I mean? the extensions done by the package on metaclass that are not in the package.

https://pharo.fogbugz.com/f/cases/16460/I-fileout-a-package-does-not-fileout-metaclass-extension

*) On windows, it appears that while the menus say Ctrl+..., they really mean Ctrl+... or Alt+... -- you figure it out.

Yes it sucks. We are in flux with the key essentially because the VM often does not even gives us the correct information.
With the new OSWindow architecture we are putting in place we will have all the events and not just the one the VM does not eat (like shift pressed)
and we can manage all at the image level not deep into arcane and old C- code. ��

*) It appears that Pharo is removing subString: in favor of substring:. The ANSI standard defines subString:.

I was not aware of it. In fact ANSI is bad and totally incomplete (for example we studies strings and this is a mess and totally incomplete).
Now we should probably add it. I will send the mail to the mailing-list and let people decide.

*) The ChunkFileFormatParser>>parseNextDeclaration is hard coded to handle only the Pharo format:
�� | substrings |
�� substrings := nextChunk parseLiterals.
�� (substrings includes: 'methodsFor:')
�� ifTrue: [ ^self parseMethodDeclarations: substrings ].
�� (substrings includes: 'commentStamp:')
�� ifTrue: [ ^self parseCommentDeclaration: substrings ].
�� (substrings includes: 'reorganize')
�� ifTrue: [ ^self parseClassOrganization: substrings ].
Due to the hard coded nature of the types of strings accepted, I have to overwrite this method to be able to file in Dolphin code. In the other Smalltalk's, I can write an extension method or two that I use to file in another dialect's code.

https://pharo.fogbugz.com/f/cases/16461/ChunkFileParser

If you have a solution please send it. Because we are focusing on the bootstrap and spur the new object format plus a new FFI. Plus a lot more.
So this keeps us really busy and many people left the team and we are restarting to teach :)

I personally hate this format and the hardcoding of these strings and selectors everywhere so this is why the ChunkFileFormatParser is already
a big step forward.

I have several other things that annoy me,

me too :)
But I tried that every couple of days I fix one.

but this is enough for now. :)

Tx john :)

John Brant

--------------------------------------------
St��phane Ducasse
http://stephane.ducasse.free.fr
http://www.synectique.eu / http://www.pharo.org
03 59 35 87 52
Assistant: Julie Jonas
03 59 57 78 50
03 59 35 86 16

S. Ducasse - Inria
40, avenue Halley,
Parc Scientifique de la Haute Borne, B��t.A, Park Plaza
Villeneuve d'Ascq 59650
France