On Fri, Apr 07, 2017 at 08:43:50PM +0200, Esteban Lorenzano wrote: > > > On 7 Apr 2017, at 20:30, Alistair Grant <[email protected]> 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
