Quantcast

FileReference / and parent

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

FileReference / and parent

Alistair Grant
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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Sven Van Caekenberghe-2
That might be OK, I guess, but you have to add enough unit tests to cover all edge cases.

In the past, the reasoning was, AFAIU, that the right side argument to #/ did not get interpreted.

But that indeed gives some inconsistencies.

> On 3 Apr 2017, at 14:04, Alistair Grant <[hidden email]> 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
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Alistair Grant
Hi Sven,

On Mon, Apr 03, 2017 at 06:06:57PM +0200, Sven Van Caekenberghe wrote:
> That might be OK, I guess, but you have to add enough unit tests to
> cover all edge cases.

I'll certainly have unit tests in place when I submit the proposed
changes.  (the current unit tests have the same results after
applying the code below).

Which edge cases are you thinking of?

Thanks!
Alistair


> In the past, the reasoning was, AFAIU, that the right side argument to
> #/ did not get interpreted.
>
> But that indeed gives some inconsistencies.
>
> > On 3 Apr 2017, at 14:04, Alistair Grant <[hidden email]>
> > 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
> >
>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Sven Van Caekenberghe-2

> On 3 Apr 2017, at 21:11, Alistair Grant <[hidden email]> wrote:
>
> Hi Sven,
>
> On Mon, Apr 03, 2017 at 06:06:57PM +0200, Sven Van Caekenberghe wrote:
>> That might be OK, I guess, but you have to add enough unit tests to
>> cover all edge cases.
>
> I'll certainly have unit tests in place when I submit the proposed
> changes.  (the current unit tests have the same results after
> applying the code below).
>
> Which edge cases are you thinking of?

As arg:

empty
/
foo/
/foo
/foo/
//
//foo
foo//
//foo//
/foo//bar/

I guess you get the idea.

> Thanks!
> Alistair
>
>
>> In the past, the reasoning was, AFAIU, that the right side argument to
>> #/ did not get interpreted.
>>
>> But that indeed gives some inconsistencies.
>>
>>> On 3 Apr 2017, at 14:04, Alistair Grant <[hidden email]>
>>> 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
>>>
>>
>>
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Stephane Ducasse-3
In reply to this post by Alistair Grant
Hi alistair

We should target Pharo 70. 
Now it is great that you help improving the file frameworks. 
Could you open a bug entry?
Do you have tests?

Stef

On Mon, Apr 3, 2017 at 2:04 PM, Alistair Grant <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Alistair Grant
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 <[hidden email]>
> 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
> >
> >

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Denis Kudriashov
Hi Alistair.

Do you open the issue?
I found that it was already exist. Look at 13217

2017-04-04 16:59 GMT+02:00 Alistair Grant <[hidden email]>:
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 <[hidden email]>
> 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
> >
> >


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Alistair Grant
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-string>

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 <[hidden email]>:
>
> > 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 <[hidden email]>
> > > 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
> > > >
> > > >
> >
> >

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

EstebanLM

> On 7 Apr 2017, at 20:30, Alistair Grant <[hidden email]> wrote:
>
> 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-string>
>
> 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.

that’s very good… thank you very much!

Esteban

>
> Thanks for letting me know about this.
>
> Cheers,
> Alistair
>
>
>
>
>> 2017-04-04 16:59 GMT+02:00 Alistair Grant <[hidden email]>:
>>
>>> 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 <[hidden email]>
>>>> 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
>>>>>
>>>>>
>>>
>>>
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Alistair Grant
On Fri, Apr 07, 2017 at 08:43:50PM +0200, Esteban Lorenzano wrote:

>
> > On 7 Apr 2017, at 20:30, Alistair Grant <[hidden email]> wrote:
> >
> > 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-string>
> >
> > 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.
>
> that's very good... thank you very much!
>
> Esteban

And of course, just after posting the proposed patch in the issue, I
found a bug with it.

After re-reading the Path class comment I've also realised that my patch
breaks the explicit behaviour in the comment.  But...

The class comment in Path (I belive) is wrong.

The class comment states that a slash (/) can be included in the file
name with "parent\/child\/", however it didn't work when I tried it, and
there are multiple answers in stackoverflow and stackexchange stating
that / and \0 (null character) are the two forbidden (ascii) characters
on linux.  Windows includes / (slash), \ (backslash) and several others.

Wikipedia has an article on it:
https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

From a purely practical point of view, I think trying to be completely
general (meaning allowing the directory separator in a filename) in this
case causes many more problems with unexpected behaviour than it solves,
so I still would like to see this changed in Pharo 7 (once I fix my
patch and extend the automated tests :-)).

Cheers,
Alistair

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: FileReference / and parent

Stephane Ducasse-3
Hi alistair

I do not remember (may be I wrote this class comment because there were none and I got frustrated). 
So improving the class comments may be needed. 
Stef

On Fri, Apr 7, 2017 at 10:31 PM, Alistair Grant <[hidden email]> wrote:
On Fri, Apr 07, 2017 at 08:43:50PM +0200, Esteban Lorenzano wrote:
>
> > On 7 Apr 2017, at 20:30, Alistair Grant <[hidden email]> wrote:
> >
> > 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-string>
> >
> > 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.
>
> that's very good... thank you very much!
>
> Esteban

And of course, just after posting the proposed patch in the issue, I
found a bug with it.

After re-reading the Path class comment I've also realised that my patch
breaks the explicit behaviour in the comment.  But...

The class comment in Path (I belive) is wrong.

The class comment states that a slash (/) can be included in the file
name with "parent\/child\/", however it didn't work when I tried it, and
there are multiple answers in stackoverflow and stackexchange stating
that / and \0 (null character) are the two forbidden (ascii) characters
on linux.  Windows includes / (slash), \ (backslash) and several others.

Wikipedia has an article on it:
https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

From a purely practical point of view, I think trying to be completely
general (meaning allowing the directory separator in a filename) in this
case causes many more problems with unexpected behaviour than it solves,
so I still would like to see this changed in Pharo 7 (once I fix my
patch and extend the automated tests :-)).

Cheers,
Alistair


Loading...