New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows's altsep support in getExt, getName and join (bugfix, I think) #63

Merged
merged 3 commits into from Jun 15, 2011

Conversation

Projects
None yet
3 participants
@denis-sh
Contributor

denis-sh commented May 28, 2011

Added Windows's altsep support to getExt, getName and join (bugfix, I think).
Some cosmetic changes.

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex May 28, 2011

I think it's simpler to just make this a non-generic function that takes a dchar. Implicit conversions will take care of char and wchar.

andralex commented on std/path.d in 254d9a8 May 28, 2011

I think it's simpler to just make this a non-generic function that takes a dchar. Implicit conversions will take care of char and wchar.

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex May 28, 2011

Same here.

andralex commented on std/path.d in 254d9a8 May 28, 2011

Same here.

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex May 28, 2011

I think isSepOrAltSep should become isSep, and isSepOrAltSepOrDriveSep should become isSepOrDriveSep.

andralex commented on 254d9a8 May 28, 2011

I think isSepOrAltSep should become isSep, and isSepOrAltSepOrDriveSep should become isSepOrDriveSep.

isSepOrAltSep => isSep
template Char type in private functions => dchar
@kyllingstad

This comment has been minimized.

Show comment
Hide comment
@kyllingstad

kyllingstad May 29, 2011

Member

Before you spend a lot more time on this, I should probably mention that I have written a new version of std.path which I intend to put up for review for inclusion in Phobos as soon as possible. It correctly handles both dir separators on Windows.

Member

kyllingstad commented May 29, 2011

Before you spend a lot more time on this, I should probably mention that I have written a new version of std.path which I intend to put up for review for inclusion in Phobos as soon as possible. It correctly handles both dir separators on Windows.

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex Jun 8, 2011

Member

Lars, what do we do this? I think it's safe to approve it.

Member

andralex commented Jun 8, 2011

Lars, what do we do this? I think it's safe to approve it.

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex Jun 8, 2011

Member

s/do this/do with this/

Member

andralex commented Jun 8, 2011

s/do this/do with this/

@kyllingstad

This comment has been minimized.

Show comment
Hide comment
@kyllingstad

kyllingstad Jun 8, 2011

Member

It's probably safe, but it may be that there isn't much point in doing so. Based on feedback I've gotten so far, I feel quite certain that my revamped std.path will be accepted in some form or other. It is ready for review now, and it should be next in line after the TempAlloc review is over. Hopefully it will be included with DMD 2.055.

At any rate, if we do approve this pull request, I just noticed two lines that should be fixed. I'll comment on them separately.

Member

kyllingstad commented Jun 8, 2011

It's probably safe, but it may be that there isn't much point in doing so. Based on feedback I've gotten so far, I feel quite certain that my revamped std.path will be accepted in some form or other. It is ready for review now, and it should be next in line after the TempAlloc review is over. Hopefully it will be included with DMD 2.055.

At any rate, if we do approve this pull request, I just noticed two lines that should be fixed. I'll comment on them separately.

Show outdated Hide outdated std/path.d Outdated
Show outdated Hide outdated std/path.d Outdated
@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex Jun 15, 2011

Member

Ping? Both '|' and '||' work here, but probably the use of '|' is a bit surprising. So let's change that to '||' and commit soon. Thanks!

Member

andralex commented Jun 15, 2011

Ping? Both '|' and '||' work here, but probably the use of '|' is a bit surprising. So let's change that to '||' and commit soon. Thanks!

@denis-sh

This comment has been minimized.

Show comment
Hide comment
@denis-sh

denis-sh Jun 15, 2011

Owner

So, and where is this commit in the pull request?

Owner

denis-sh commented on d3c7cfd Jun 15, 2011

So, and where is this commit in the pull request?

@denis-sh

This comment has been minimized.

Show comment
Hide comment
@denis-sh

denis-sh Jun 15, 2011

Contributor

Pushed "ping accepted" commit denis-sh/phobos@d3c7cfd to denis-sh:patch-3. No idea where is it.

Contributor

denis-sh commented Jun 15, 2011

Pushed "ping accepted" commit denis-sh/phobos@d3c7cfd to denis-sh:patch-3. No idea where is it.

@andralex

This comment has been minimized.

Show comment
Hide comment
@andralex

andralex Jun 15, 2011

Member

Hm. Let me pull this now. No need to make the change in a subsequent pull request (I think we're okay either way). Thanks!

Member

andralex commented Jun 15, 2011

Hm. Let me pull this now. No need to make the change in a subsequent pull request (I think we're okay either way). Thanks!

andralex added a commit that referenced this pull request Jun 15, 2011

Merge pull request #63 from denis-sh/patch-3
Windows's altsep support in getExt, getName and join (bugfix, I think)

andralex added a commit that referenced this pull request Jun 15, 2011

Merge pull request #63 from denis-sh/patch-3
Windows's altsep support in getExt, getName and join (bugfix, I think)

@andralex andralex merged commit 37763e3 into dlang:master Jun 15, 2011

NilsBossung pushed a commit to NilsBossung/phobos that referenced this pull request Jun 15, 2013

Merge pull request dlang#63 from thekvs/master
Check for existence of  'objs' directory and create one if it doesn't exist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment