www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.path review: final version(?)

reply "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
Since the voting is supposed to start today (not in this thread; dsimcha 
will annouce it), here is the (hopefully) final version of std.path:

  https://github.com/kyllingstad/phobos/blob/std-path/std/path.d
  http://www.kyllingen.net/code/std-path/phobos-prerelease/std_path.html

New commits since the last update (dated 2011-08-03 and 2011-08-04) are 
listed here:

  https://github.com/kyllingstad/phobos/commits/std-path

Highlights:

  - extension() now returns the dot together with the extension
  - The dot is optional in set-/defaultExtension()
  - Removed normalize(), since it simply called buildNormalizedPath()

I kept the longer names for *Extension and *Separator, because

  - I personally like them better; they are clear and consistent
  - Jonathan is the only one who seems to really dislike them
  - There is ample precedent for longish function names in Phobos (see 
std.algorithm, for instance)

-Lars
Aug 04 2011
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday 04 August 2011 10:55:31 Lars T. Kyllingstad wrote:
 Since the voting is supposed to start today (not in this thread; dsimcha
 will annouce it), here is the (hopefully) final version of std.path:
 
   https://github.com/kyllingstad/phobos/blob/std-path/std/path.d
   http://www.kyllingen.net/code/std-path/phobos-prerelease/std_path.html
 
 New commits since the last update (dated 2011-08-03 and 2011-08-04) are
 listed here:
 
   https://github.com/kyllingstad/phobos/commits/std-path
 
 Highlights:
 
   - extension() now returns the dot together with the extension
   - The dot is optional in set-/defaultExtension()
   - Removed normalize(), since it simply called buildNormalizedPath()
 
 I kept the longer names for *Extension and *Separator, because
 
   - I personally like them better; they are clear and consistent
   - Jonathan is the only one who seems to really dislike them
   - There is ample precedent for longish function names in Phobos (see
 std.algorithm, for instance)

As a reminder, before you create a pull request for the new std.path (which looks like it'll probably make it in via a unanimous vote in favor), you need to replace the aliases for the old functions with the actual old functions (since the behavior has changed for a number of them) and mark them as scheduled for deprecation in their documentation instead of deprecating them. - Jonathan M Davis
Aug 06 2011
prev sibling next sibling parent "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
On Sat, 06 Aug 2011 14:25:40 -0700, Jonathan M Davis wrote:

 On Thursday 04 August 2011 10:55:31 Lars T. Kyllingstad wrote:
 Since the voting is supposed to start today (not in this thread;
 dsimcha will annouce it), here is the (hopefully) final version of
 std.path:
 
   https://github.com/kyllingstad/phobos/blob/std-path/std/path.d
   http://www.kyllingen.net/code/std-path/phobos-prerelease/


 
 New commits since the last update (dated 2011-08-03 and 2011-08-04) are
 listed here:
 
   https://github.com/kyllingstad/phobos/commits/std-path
 
 Highlights:
 
   - extension() now returns the dot together with the extension - The
   dot is optional in set-/defaultExtension() - Removed normalize(),
   since it simply called buildNormalizedPath()
 
 I kept the longer names for *Extension and *Separator, because
 
   - I personally like them better; they are clear and consistent -
   Jonathan is the only one who seems to really dislike them - There is
   ample precedent for longish function names in Phobos (see
 std.algorithm, for instance)

As a reminder, before you create a pull request for the new std.path (which looks like it'll probably make it in via a unanimous vote in favor), you need to replace the aliases for the old functions with the actual old functions (since the behavior has changed for a number of them) and mark them as scheduled for deprecation in their documentation instead of deprecating them.

Thanks for the reminder. I'll make sure to do this. -Lars
Aug 07 2011
prev sibling parent reply Rainer Schuetze <r.sagitario gmx.de> writes:
I like how std.path has turned out. Thanks, Lars.

Two minor issues while skipping through the source (I've been on 
vacation, and it took some time to catch up, so I'm not sure these have 
already been discussed):

- C[] baseName(C, C1)(C[] path, C1[] suffix) does not respect the case 
sensitivity of the OS with respect to suffix, I think it should.

- string absolutePath(string path, string base) returns an empty string 
if path is empty. I would have expected that the base directory is 
returned, so that you'll always get a sensible path.

Rainer

On 04.08.2011 12:55, Lars T. Kyllingstad wrote:
 Since the voting is supposed to start today (not in this thread; dsimcha
 will annouce it), here is the (hopefully) final version of std.path:

    https://github.com/kyllingstad/phobos/blob/std-path/std/path.d
    http://www.kyllingen.net/code/std-path/phobos-prerelease/std_path.html

 New commits since the last update (dated 2011-08-03 and 2011-08-04) are
 listed here:

    https://github.com/kyllingstad/phobos/commits/std-path

 Highlights:

    - extension() now returns the dot together with the extension
    - The dot is optional in set-/defaultExtension()
    - Removed normalize(), since it simply called buildNormalizedPath()

 I kept the longer names for *Extension and *Separator, because

    - I personally like them better; they are clear and consistent
    - Jonathan is the only one who seems to really dislike them
    - There is ample precedent for longish function names in Phobos (see
 std.algorithm, for instance)

 -Lars

Aug 12 2011
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday, August 12, 2011 10:14:11 Rainer Schuetze wrote:
 I like how std.path has turned out. Thanks, Lars.
 
 Two minor issues while skipping through the source (I've been on
 vacation, and it took some time to catch up, so I'm not sure these have
 already been discussed):
 
 - C[] baseName(C, C1)(C[] path, C1[] suffix) does not respect the case
 sensitivity of the OS with respect to suffix, I think it should.
 
 - string absolutePath(string path, string base) returns an empty string
 if path is empty. I would have expected that the base directory is
 returned, so that you'll always get a sensible path.

Except, you gave it an invalid path if you gave it an empty string. Such a bug would be found faster if you get an invalid path back than if you get the base directory. I definitely think that if you give an invalid path to a function, it's better to get an invalid path back out. Otherwise, it's just hiding bugs, making them hard to catch and debug. - Jonathan M Davis
Aug 12 2011
parent Rainer Schuetze <r.sagitario gmx.de> writes:
On 12.08.2011 10:31, Jonathan M Davis wrote:
 On Friday, August 12, 2011 10:14:11 Rainer Schuetze wrote:
 - string absolutePath(string path, string base) returns an empty string
 if path is empty. I would have expected that the base directory is
 returned, so that you'll always get a sensible path.

Except, you gave it an invalid path if you gave it an empty string. Such a bug would be found faster if you get an invalid path back than if you get the base directory. I definitely think that if you give an invalid path to a function, it's better to get an invalid path back out. Otherwise, it's just hiding bugs, making them hard to catch and debug.

I don't have a very strong opinion on this, so I'm also fine with the current state. The use case I was thinking of was treating some user setting, where it might be bothersome to always specify "." instead of leaving a setting blank. Also, when being used script like, it might be desirable to avoid the necessity for error handling to keep user code short by making best guesses at what the caller expects, as long as it is well-defined and in the documentation. I know this is a little late, but speaking of invalid paths, a small addition to std.path could be a function to check whether a string is made up of valid path characters (as given in http://msdn.microsoft.com/en-us/library/aa365247%28v=vs.85%29.aspx for windows) and a function that converts/eliminates these characters from a path.
Aug 13 2011
prev sibling next sibling parent "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
On Fri, 12 Aug 2011 10:14:11 +0200, Rainer Schuetze wrote:

 I like how std.path has turned out. Thanks, Lars.
 
 Two minor issues while skipping through the source (I've been on
 vacation, and it took some time to catch up, so I'm not sure these have
 already been discussed):
 
 - C[] baseName(C, C1)(C[] path, C1[] suffix) does not respect the case
 sensitivity of the OS with respect to suffix, I think it should.

Good point. I'll fix.
 - string absolutePath(string path, string base) returns an empty string
 if path is empty. I would have expected that the base directory is
 returned, so that you'll always get a sensible path.

I'll consider it, but I think I agree with Jonathan on this one. -Lars
Aug 13 2011
prev sibling parent "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
On Sat, 13 Aug 2011 16:53:39 +0200, Rainer Schuetze wrote:

 I know this is a little late, but speaking of invalid paths, a small
 addition to std.path could be a function to check whether a string is
 made up of valid path characters (as given in
 http://msdn.microsoft.com/en-us/library/aa365247%28v=vs.85%29.aspx for
 windows) and a function that converts/eliminates these characters from a
 path.

You're in luck. Andrei also requested such a function, so I have already written it. ;) I'll push it to my repo, together with a few other commits, some time tomorrow. -Lars
Aug 13 2011