filetree icon indicating copy to clipboard operation
filetree copied to clipboard

Method header incompatible with STIG

Open cdlm opened this issue 9 years ago • 10 comments

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.

cdlm avatar Feb 05 '15 13:02 cdlm

@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:)

dalehenrich avatar Feb 06 '15 02:02 dalehenrich

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?

cdlm avatar Feb 06 '15 10:02 cdlm

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:)

dalehenrich avatar Feb 06 '15 18:02 dalehenrich

On 6 February 2015 at 19:09, Dale Henrichs [email protected] 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

cdlm avatar Feb 11 '15 18:02 cdlm

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...

dalehenrich avatar Feb 11 '15 20:02 dalehenrich

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 avatar Mar 04 '15 16:03 cdlm

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 ?

cdlm avatar Mar 06 '15 15:03 cdlm

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

dalehenrich avatar Mar 06 '15 17:03 dalehenrich

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

ThierryGoubier avatar Mar 06 '15 17:03 ThierryGoubier

See #151.

cdlm avatar Mar 06 '15 18:03 cdlm