The Trunk: Files-tpr.123.mcz

tim Rowledge uploaded a new version of Files to project The Trunk:
http://source.squeak.org/trunk/Files-tpr.123.mcz

==================== Summary ====================

Name: Files-tpr.123
Author: tpr
Time: 21 June 2013, 5:40:33.465 pm
UUID: b3983780-394c-464b-8924-b70282415bfe
Ancestors: Files-fbs.122

Fix bug reported in mantis ttp://bugs.squeak.org/view.php?id=7740
FileTestUrl should now pass

=============== Diff against Files-fbs.122 ===============

Item was changed:
  ----- Method: AcornFileDirectory>>pathParts (in category 'path access') -----
  pathParts
  "Return the path from the root of the file system to this directory as an
  array of directory names.
  This version tries to cope with the RISC OS' strange filename formatting;
  filesystem::discname/$/path/to/file
  where the $ needs to be considered part of the filingsystem-discname atom."
  | pathList |
  pathList := super pathParts.
  (pathList indexOf: '$') = 2
  ifTrue: ["if the second atom is root ($) then stick $ on the first atom
  and drop the second. Yuck"
+ ^ pathList species
- ^ Array
  streamContents: [:a |
  a nextPut: (pathList at: 1), '/$'.
  3 to: pathList size do: [:i | a
  nextPut: (pathList at: i)]]].
  ^ pathList!

On 22 June 2013 01:40,  <[hidden email]> wrote:

Gold star to Tim! Actual bug reports! This is how it should be done!
(Also, in case that sounds patronising I really don't mean it to be. I
mean it in the "yay, someone's doing exactly the right thing" tone of
voice, and I believe in telling people when they do the right thing.)

Regarding your note in the ticket, we won't have a tag for 4.5 because
"4.5" means "bugs in the released 4.5". Since 4.5 hasn't been
released, bugs in the current trunk just get the "trunk" label.

frank

On Sat, 22 Jun 2013, Frank Shearar wrote:

Almost :). The status should be resolved, not closed. We used to close
the resolved issues after a new release (and that's when we used to change
the version tag to an actual version number from "trunk", because
"trunk" is intended to be used for issues in the current trunk). I
think this hasn't happened yet with the 4.4 release.


Levente

On Sat, 22 Jun 2013, [hidden email] wrote:

If pathList is always an OrderedCollection (which is true), then
#streamContents: is not needed. If it's always a new object (which is
also true), then there's no need to copy it. So I think the following
would be better:

pathParts

  | pathList |
  pathList := super pathParts.
  (pathList at: 2 ifAbsent: [ nil ]) = '$' ifTrue: [
  pathList addFirst: pathList removeFirst, self slash, pathList removeFirst ].
  ^pathList

I can't test it, so it's up to you. I removed the comments, but they should be kept.


Levente

> - ^ Array
>   streamContents: [:a |
>   a nextPut: (pathList at: 1), '/$'.
>   3 to: pathList size do: [:i | a
>   nextPut: (pathList at: i)]]].
>   ^ pathList!
>
>
>

On 22-06-2013, at 5:11 AM, Levente Uzonyi <[hidden email]> wrote:

That is certainly cleaner - but as you said *if* pathList is always an OC. The reason the older code had to cope with an Array is that it used to get an Array until someone changed it. This is a common problem that is quite hard to deal with, where users of a message need changing to match the implementation. We ought to be able to do better but it's hard. You could almost have some sympathy with the idea of specifying interfaces. Yuck.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Useful Latin Phrases:- Re vera, potas bene = Say, you sure are drinking a lot.