Hi Dennis, On Fri, Apr 07, 2017 at 10:58:57AM +0200, Denis Kudriashov wrote:
Hi Alistair.
Do you open the issue? I found that it was already exist. Look at 13217 <https://pharo.fogbugz.com/f/cases/13217/FS-basename-with-compound-path-strin...>
I haven't opened it yet - as you suggested in the issue, given how busy Esteban and everyone is with getting Pharo 6 out the door, I'm waiting for Pharo 7 dev to start. I've got the code and automated tests done, although reading the issue has made me think that I should also review the class comments. I'll add a comment to the issue and then submit the patch once the Pharo 7 inbox is open. Thanks for letting me know about this. Cheers, Alistair
2017-04-04 16:59 GMT+02:00 Alistair Grant <akgrant0710@gmail.com>:
Hi Stef,
On Tue, Apr 04, 2017 at 04:33:56PM +0200, Stephane Ducasse wrote:
Hi alistair
We should target Pharo 70.
Yep, I saw Esteban's message saying he hopes 6.0 is only a week away, so this can definitely wait.
Now it is great that you help improving the file frameworks. Could you open a bug entry?
Yep, I was just waiting to see if there was lots of resistance to these changes (which there doesn't seem to be).
Do you have tests?
I was working on them when this email arrived. :-)
I expect it will take me a week to get them to the point I'm happy (this is a part time hobby), but they'll definitely be part of the slice / pull request when I submit it.
Cheers, Alistair
Stef
On Mon, Apr 3, 2017 at 2:04 PM, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi All,
I previously suggested a change to Path>>/ which actually covered two issues:
1. The handling of the parent directory notation, i.e. ".." 2. The construction of path segments when appending a string.
As Damien pointed out, the first issue needs a bit more consideration.
I think the second point is still problematic and can be addressed separately. In particular:
('/a/b/c' asFileReference / 'd/e/f') parent "File @ /a/b/c"
I would expect the result to be "File @ /a/b/c/d/e"
The fix is straightforward (although someone may be able to propose a more elgant solution):
-- / aString | path additionalPath index |
aString isEmptyOrNil ifTrue: [ Error signal: 'Path element cannot be empty
or
nil'].
additionalPath := Path from: aString. path := self class new: self size + additionalPath size. path copyFrom: self. index := self size + 1. additionalPath do: [ :each | path at: index put: each. index := index + 1. ]. ^ path --
1. Do you agree with the proposed change? 2. (Assuming you agree): Should we target Pharo 6.0 or 7.0? On one side, this is clearly a bug, on the other, no one has reported it to date, so it isn't having a big impact.
Thanks, Alistair