pharo-vcs / tonel Goto Github PK
View Code? Open in Web Editor NEWLicense: MIT License
License: MIT License
I still don't have Tonel ported yet, so I don't have a working example to play with ... however I am curious if and where you guarantee UTF8 encoding of the file contents ... I didn't notice anything in the tests that deal with UTF8 encoded source
Should be really commented and type of arguments should be much more precise.
And sketch of an example should also be added.
Just stepping through the parser as it reads meta data and I see that it is simply counting {
and }
, so it is vulnerable to either one of those characters embedded in the meta data itself (including symbols) ...
There was some confusion because Tonel uses STON to represent meta data and STON has a slightly different idea of literal (unquoted) Symbols than standard Pharo.
Since Tonel already subclasses STONWriter, the following change would improve the situation.
TonelSTONWriter>>#isSimpleSymbol: symbol
"Customize STON to only consider very clean symbols as literal,
for all others err on the safe side and quote them."
symbol isEmpty
ifTrue: [ ^ false ].
('abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' includes: symbol first)
ifFalse: [ ^ false ].
^ symbol allSatisfy: [ :each |
'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' includes: each ]
right now they are as extensions of metacello but this is a problem because tonel may be loaded after it (and in general is like that).
Currently, you cannot load a project in OSX if the sources are on a shared drive or external drive because OSX generates hidden metadata files that tonel tries to load.
I'll do a PR to ignore those files.
For FileTree I have found it necessary to put in validation checks that the method protocol for class extensions matches the package name .... human beings will be editing the files by hand and they will be cutting and pasting, so these kind of detail errors will happen ... the consequence of not validating is that the package structure will be immediately corrupted once the incorrect protocol names are loaded into the system ... same thing should probably go for regular classes where the method protocol needs to NOT be a package name ...
Class categories are the same ... the category of the class has to match the name of the package ...
Only two tests are being run, but there are a number of other tests that could be inherited.
I think that shouldInheritSelectors
needs to be implemented to return true
on the class-side to run the tests ...
We can deal with filetree based history by replaying the commits.
Taken from pharo-vcs/iceberg#568
origin issue: https://pharo.fogbugz.com/f/cases/20659
When I accidentally encountered the code of TonelParser>>methodBody
a voice in my head told: it's too complex to be honest
So I decided to see how long it would take to attack...
Less than 5 minutes, see this test:
testMethodBodyWithSquareBracketInsideLiteralArray
self
assertParse: '[ "What about this valid literal Array guys" ^#( [ ) ]'
rule: #methodBody
equals: ' "What about this valid literal Array guys" ^#( [ ) '.
If ever you manage to fix it, I have more for you
testMethodBodyWithCommentNextToDollarCharLiteral
self
assertParse: '[^$$"what about this one]"]'
rule: #methodBody
equals: '^$$"what about this one]"'.
I would change the following method
TonelWriter>>#toSTON: anObject
^ String streamContents: [ :out |
(TonelSTONWriter on: out)
newLine: self newLine;
nextPut: anObject ]
Up on travis it seems that when I am loading my brand new package structure the image is getting confused and it seems that it might be due to the fact that Tonel is already in the image ... now this is Pharo7.0 so there might be bugs, but I would have expected the undeclared global in that method to be resolved after the package that contained the class was loaded ...
Pharo6.1 has a similar set of warnings, but the test failures appear to be of a more reasonable variety ... maybe it's premature to include Pharo7.0 in the travis tests?
TonelWriter class>>#exportClass:on: is very inefficient. It does o snapshot of all the package before which may be very slow:
[
String streamContents: [:s |TonelWriter exportClass: Object on: s ]
] timeToRun "0:00:00:04.785"
We could use more direct approach, something like
[
writer := TonelWriter new.
class := Object.
definition := class asClassDefinition.
snapshot := MCSnapshot fromDefinitions: {definition}, (class localMethods collect: [:each | each asRingDefinition asMCMethodDefinition ]), (class classSide localMethods collect: [:each | each asRingDefinition asMCMethodDefinition]).
writer instVarNamed: #snapshot put: snapshot.
String streamContents: [ :aStream |
writer writeClassDefinition: definition on: aStream.
writer writeClassSideMethodDefinitions: definition on: aStream.
writer writeInstanceSideMethodDefinitions: definition on: aStream ]
] timeToRun " 0:00:00:00.11"
when exporting a class with traits, I have this output:
Class {
#name : #BlPoint,
#superclass : #Object,
#traits : 'TBlDebug',
#classTraits : 'TBlDebug classTrait',
#category : 'Bloc-Basic-Math'
}
The classTraits
section is redundant (and is actually not needed to rebuild the class). This section has sense when there are rules that modifies the trait composition, but otherwise is cleaner not including it.
I start noticed that instance and class side extensions with same selector are moving between commits.
The reason is that tonel writer sorts extension methods only by selector but it should also consider the class
When you commit packages in Tonel format for Pharo 6.1 and 7 the output is not the same.
For example:
#category : #'Boardwalk-Tests-Javascript'
#category : 'Boardwalk-Tests-Javascript'
A version gets the category as a String and the other as a Symbol.
Following test fails:
TonelParserTest>>testMethodBodyWithDollarAndBracket
self
assertParse: '[ [ $$] ]'
rule: #methodBody
equals: ' [ $$] '.
It shows problem with methods which return dollar character from block:
AClass>>someMethod
true ifTrue: [^$$]
Notice that space between dollar and bracket works fine
the method change is lost, I will bring it back
A trait can use special slots.
Today only the slot names are exported.
The package Moose-TestResources-KGB-P11FullReferee contains only the method extensions. The method TonelWriter>>#writeSnapshot:
does following:
self writePackage: (snapshot definitions
detect: #isOrganizationDefinition
ifFound: [ :each | each ]
ifNone: [ self createDefaultOrganizationFrom: (snapshot definitions detect: #isClassDefinition) ]).
The snapshot does contain no organization nor class definition (it contains only 3 method definitions) so the code fails.
The implication of this is that every time someone on windows saves a packages all of the involved classes will show up with a host of line ending changes (I imagine) which would not be a good thing ... the format really needs to avoid spurious diffs as much as possible
We are writing a GUI for Exercism.io to submit solutions directly from Pharo to their web server,
so we need to generate Tonel files as strings without touching the file system.
Refactor to facilitate this.
[EDIT: I forgot to mention, I'll submit a PR for this soon]
older Pharo versions do not initialise variables for trait definition, so we have a DNU
because there is no class organisation
Fixed in #21
I've just forked tonel to https://github.com/GsDevKit/tonel with the plan to port tonel to GemStone base image over the next few weeks. Since tonel is ulitimately intended to be a cross-platform file format I think a cross platform project would be preferred --- using travis to valid changes against ... a common package structure, etc. --- Presumably there is a fair amount of code that can be shared between platforms -- but I admit that I haven't delved too deep into the code to see just how portable it currently is...
If there is interest in making the pharo-vcs-tonel project cross platform then I will try to partition the packages/code while porting to the GemStone base image and to the GsDevKit project (upon which Seaside is based).
If there is no interest in hosting a cross-platform project, then I will undertake that task and break the forked relationship as there will be little in common other than a common starting point ...
Is there a good reason why Tonel uses platform-specific line ending?
TonelWriter>>newLine
^ OSPlatform current lineEnding
It produces a lot of problems, and I don't see any advantage.
During ESUG (2017, so more than a year) we agreed a format for Tonel files that is slightly different from the one existing now.
We agreed:
This easies some parsing, etc.
#writeClass: and #writePackage: aren't listed as private and yet they can't be used standalone as they internally call #writeClassSideMethodDefinitions: aClassDefinition on: aStream and it references a snapshop.
Would be useful to perform these operations - and have some tests for them.
After 20 min looking for it I still do not find it.
I'm stupid for sure.
TonelWriter>>#createDefaultOrganizationFrom:
should return an MCOrganization but currently it can return a string (a category name). The code should be:
createDefaultOrganizationFrom: aCollection
"answers a default organisation for the cases where there are none"
"simplest case, I answer the clas definition"
snapshot definitions
detect: #isClassDefinition
ifFound: [ :each | ^ MCOrganizationDefinition categories: { each category } ].
^ self createDefaultOrganizationFromDefinition: (snapshot definitions
detect: #isMethodDefinition
ifNone: [ self error: 'cannot determine package name from empty snapshot' ])
But this migration script is really great! Thanks a lot, I will convert all my gitfiletree projects to Iceberg+Tonel. :-)
The current on:fileName: API is bad. We should be able to do
(TonelReader new filenamed: 'ffo.st') loadDefinitions.
When there is a trait that uses another trait with class-side method, but uses exclusion or aliasing for that class-side method, the exclusion and/or alias is not saved into file.
Way to reproduce:
Tested in Pharo 6.1 (60546) with Iceberg 1.4.1
Would like to write a package out to a different directory name (one base on the tag) - this isn't easy due to write package not using a setter (if it used self packageDir: I could subclass and override this).
writePackage: aDefinition
"I'm assuming first category is actually the package"
packageDir := aDefinition categories first.
Could we use a setter - or let you specifify a name in some way.
This would be handy for non git related usages (e.g. exercism)
I encountered a problem regarding the import of tonel repositories: It throws the TonelParseError 'Can''t parse method body'. To reproduce, simply create a Package containing a single Class with a single method. Source of that method is: [$$]
The export of the method works fine and the files look okay, but the import crashes.
Note: it works when using [ $$ ]
instead of [$$]
we have a PR that changed a class category to #category : #'FuelPlatform-Pharo-Core-FuelPlatform-Pharo-Core
. Original category name was FuelPlatform-Pharo-Core
. In Tonel the class is placed in a right directory named FuelPlatform-Pharo-Core
. But in the image that was bootstrapped from this PR, the class is in a strange state:
instanceVariableNames: ''
classVariableNames: ''
package: 'FuelPlatform-Pharo-Core-FuelPlatform-Pharo-Core'
(RPackageOrganizer default packages includes: FLPharoPlatform package) >>> false
In the latest Pharo 7, I get an error while loading some files.
The file concerned is:
The error is:
TonelParser>>type
TonelParser>>typeDef
TonelParser>>document
TonelParser>>start
TonelParser class>>parseStream:
[ :s | TonelParser parseStream: s ] in [ :each | each readStreamDo: [ :s | TonelParser parseStream: s ] ] in TonelReader>>loadDefinitions in Block: [ :s | TonelParser parseStream: s ]
[ aBlock value: stream ] in FileReference(AbstractFileReference)>>readStreamDo: in Block: [ aBlock value: stream ]
BlockClosure>>ensure:
FileReference(AbstractFileReference)>>readStreamDo:
FileSystemDirectoryEntry>>readStreamDo:
[ :each | each readStreamDo: [ :s | TonelParser parseStream: s ] ] in TonelReader>>loadDefinitions in Block: [ :each | each readStreamDo: [ :s | TonelParser pa...etc...
Array(SequenceableCollection)>>collect:
TonelReader>>loadDefinitions
TonelReader(MCVersionReader)>>definitions
TonelReader(MCVersionReader)>>snapshot
[ self snapshot ] in TonelReader(MCVersionReader)>>basicVersion in Block: [ self snapshot ]
MCVersion>>snapshot
[ :ea |
ea canOptimizeLoading
ifTrue: [ ea patch applyTo: loader ]
ifFalse: [ loader updatePackage: ea package withSnapshot: ea snapshot ] ] in MCVersionLoader>>basicLoadWithNameLike: in Block: [ :ea | ...
OrderedCollection>>do:
MCVersionLoader>>basicLoadWithNameLike:
[ self basicLoadWithNameLike: aString ] in MCVersionLoader>>loadWithNameLike: in Block: [ self basicLoadWithNameLike: aString ]
[ returnValue := aBlock value ] in [ [ returnValue := aBlock value ]
ensure: [ self announceLoadStop: aString ] ] in MCVersionLoader>>announceLoad:do: in Block: [ returnValue := aBlock value ]
BlockClosure>>ensure:
[ [ returnValue := aBlock value ]
ensure: [ self announceLoadStop: aString ] ] in MCVersionLoader>>announceLoad:do: in Block: [ [ returnValue := aBlock value ]...
BlockClosure>>ensure:
MCVersionLoader>>announceLoad:do:
MCVersionLoader>>loadWithNameLike:
[ self ensurePackage: version package.
self loadWithNameLike: version info name ] in [ | version |
version := versions first.
[ self ensurePackage: version package.
self loadWithNameLike: version info name ] asJob
title: 'Loading ' , version info name asString;
run ] in MCVersionLoader>>load in Block: [ self ensurePackage: version package....
BlockClosure>>cull:
[ ^ block cull: self ] in [ self prepareForRunning.
CurrentJob value: self during: [ ^ block cull: self ] ] in Job>>run in Block: [ ^ block cull: self ]
When I try to debug I see that it check if "ass {" matches "Class". I suspect that the problem is here because this class does not have a comment. Maybe the recent Tonel generates ""
when there is no comment, but since I did not write the previous file by hand I think at one time it generated nothing.
Maybe Tonel should manage this case to be retro compatible?
Looking at initial test errors turns up a reference to OsPlatform ... haven't look for others.
The package MonticelloTonel-FileSystem
is quasi-platform specific ... at least until GemStone starts using FileSystem, so there appears to be the need for another mechanism ... perhaps it makes sense to make a platform-specific method -- isolating the reference to OsPlatform
in it's own platform-specific method and then putting that method in a platform-specific package. ... this approach would make the most sense unless there are a lot of platform-specific references ...
Anyway, @estebanlm I am game for whatever approach you want to use ...
seems that the end of line exporting has problems since some invisible chars appears on end of method
We create the file in the package directory ... presumably to preserve the name of the package ... or are we using the name of the directory for the name of the package (appears to be doing that now) ... so the package.st
is either a unnecessary appendage, or we need to read the package name from the file or .... is something else planned for that file?
In my porting work I am going to add more packages to Tonel to support the fact that the base image in GemStone doesn't use Monticello, so the packages will have to refactored ...
By not having the packages isolated in a subdirectory, the home directory of the project starts to get cluttered with .package directories and other directories that might be created ... I may want to include tode/gemstone directories with platform specific scripts ...
Pushing the packages down a level would relieve the clutter ... this is pretty much a standard practice AFAICT, so perhaps there is some other reason for not putting the packages into a subdirectory?
Is there a reason that this is not a Metadataless repo? I'm now in the process of porting Tonel to the base of GemStone and there is no Monticello implementation present (using Cypress1 now an Cypress2 eventually) so I won't able to write the packages out and preserve the monticello metadata ... if it is needed, I understand, but if it isn't I'd like to nuke the metadata now as it will be much easier for me:)
Following tests are failed:
testWriteMethodDefinitionWithStrangeComment
| writer def stream |
writer := TonelWriter new.
stream := String new writeStream.
def := MCMethodDefinition
className: #Object
classIsMeta: false
selector: #a:b:
category: 'accessing'
timeStamp: nil
source:
'a: "b:" arg1 b: arg2
^ 42'.
writer writeMethodDefinition: def on: stream.
self
assert: stream contents
equals:
('
{ #category : #accessing }
Object >> a: "b:" arg1 b: arg2 [
^ 42
]
' withLineEndings: OSPlatform current lineEnding)
Problem with algorithm which extracts method name line from sources:
TonelWritter>>selectorIsComplete: keywords in: aString
| start |
start := 1.
keywords
do: [ :each |
| index |
index := aString
findString: each
startingAt: start
caseSensitive: true.
index = 0
ifTrue: [ ^ false ].
start := index + each size ].
^ true
I found that classes with nil superclass are written with #nil symbol instead of simple nil.
Is it intentional? If yes then close issue.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.