dalehenrich / filetree

Monticello repository for directory-based Monticello packages enabling the use of git, svn, etc. for managing Smalltalk source code.
https://github.com/CampSmalltalk/Cypress
MIT License
133 stars 26 forks source link

Method header incompatible with STIG #144

Open cdlm opened 9 years ago

cdlm commented 9 years ago

This seems related to #62, and definitely to CampSmalltalk/Cypress#18

FileTree expects method files to look like

category
selector: arg and: arg
    ^ body

But STIG starts the file with a notice/category comment instead of the bare category name before the method code.

dalehenrich commented 9 years ago

@cdlm ... technically both formats are correct... FileTree has not been updated to support the newer Cypress format ... I just don't have the time:), especially when the current format is "working just fine." Pull requests are welcome:)

cdlm commented 9 years ago

So STIG is the closest to the agreed-upon format?

In fact I'm more concerned about making incompatible changes that will prevent people who already use FileTree from loading their code. I saw there is a notion of layout, would this be a good direction to look into? Do repos/commits have their layout automatically detected?

dalehenrich commented 9 years ago

Well, the current FileTree format (first-line method protocol) is also an "agreed-upon" format. The STIG version is a newer format...I'd expect that a platform would support both formats ...

In fact I'm more concerned about making incompatible changes that will prevent people who already use FileTree from loading their code.

as you point out there are quite a few "old-format" repositories out there:) and I share your concerns.

I saw there is a notion of layout, would this be a good direction to look into? Do repos/commits have their layout automatically detected?

For Filetree repos, layout detection should be done by looking at the .filetree property file. In the newer version of Cypress, a property file is explicitly called for, but for Filetree I would expect to retain the .filetree file for backwards compatibility ... Also if you poke around in the code you will find that there are already several prior versions of the Filetree format still supported:

If you're interested in working on new support for the new version I would be inclined to suggest that you start with a fresh set of classes and a fresh approach and not try to reuse a bunch of code ... then the old hierarchy can be easily retired some day:)

cdlm commented 9 years ago

On 6 February 2015 at 19:09, Dale Henrichs notifications@github.com wrote:

If you're interested in working on new support for the new version I would be inclined to suggest that you start with a fresh set of classes and a fresh approach and not try to reuse a bunch of code ... then the old hierarchy can be easily retired some day:)

Which hierarchy ?

I have the FileTree code that is loaded by default in Pharo 4, without unit tests.

I've looked into loading the tests, but I'm confused about which branch to load. Trying to load Metacello Preview broke Monticello, and loading the master branch of FileTree from github seems to rely on broken code (out of sync with FileSystem I suppose… the error I got was that the #zip method calls #closed on something that is supposed to be a stream, but is really a FileReference)

Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet

dalehenrich commented 9 years ago

Ah yes, the load instructions aren't clear... for Pharo3.0 and Pharo4.0, the following:

Metacello new
  baseline: 'FileTree';
  repository: 'github://dalehenrich/filetree:pharo3.0/repository';
  load: 'Tests'.

If you want/need the latest version of Metacello, do the following:

Metacello new
  baseline: 'Metacello';
  repository: 'github://dalehenrich/metacello-work:master/repository';
  get.
Metacello new
  baseline: 'Metacello';
  repository: 'github://dalehenrich/metacello-work:master/repository';
  onConflict: [:ex | ex allow];
  load

These were the classes that I was specifically thinking about junking for the new format...:

MCFileTreeAbstractReader
 MCFileTreeStReader
 MCFileTreeStSnapshotReader
  MCFileTreeStCypressReader
MCFileTreeAbstractStWriter
 MCFileTreeStSnapshotWriter
  MCFileTreeStCypressWriter
 MCFileTreeStWriter

I imagine that the two abstract classes have crift in them to support 5 or 6 different repository formats leading up the present format ... The Cypress in the name refers to the older Cypress format ...

I think that with a fresh start perhaps aimed at supporting both formats and relegating the previous formats to obsucurity (code wise).

The test repositories directory has examples of all of the different formats support by the current hierarcy...

cdlm commented 9 years ago

Progress report: I can browse a package coming from VW using the Monticello Browser :)

I installed a new MCCypressRepository to keep the current one useable, and a MCFileTreeReader into which I lazily copy methods from MCFileTreeStCypressReader. I'm extracting smaller methods and factoring stuff one the way… I guess we'll eventually have to check for portability, but at the moment my goal is simplicity and readability of the code.

cdlm commented 9 years ago

I've pushed my code as-is on https://github.com/cdlm/filetree/tree/cypress-compatibility-refactor I'm thinking of opening a pull-request more with the goal of reviewing code than merging, at least in the short term. What do you think ?

dalehenrich commented 9 years ago

Yeah, that's not a bad idea ... it will make much easier to manage the code review ... go for it!

ThierryGoubier commented 9 years ago

I certainly would like to have a look at it :)

cdlm commented 9 years ago

See #151.