getCurrentDir should be in ospaths, not os

[timotheecour]

/cc @demotomohiro @Araq @dom96

a number of procs are in os instead of ospaths just because getCurrentDir is defined in os instead of ospaths, eg:

• proc absolutePath*(path: string, root = getCurrentDir()) (because of getCurrentDir)
• normalizedPath (because it depends on isAbsolute)
• it also prevents having an overload of relativePath with 1 param as found in lots of languages (D, python, boost), as it would then force relativePath to be in os instead of ospaths (see https://github.com/nim-lang/Nim/pull/8166#issuecomment-433579071 for that point)

Furthermore, we already use getEnv to access environment variables in ospaths, so using the low-level c procs getcwd/getCurrentDirectoryW wouldn't be an issue IMO

proposal

move getCurrentDir, absolutePath, normalizedPath to ospaths this would have zero impact on existing code

[dom96]

ospaths should be merged back into os.

[krux02]

Just make sure to always import both, os and ospaths. I am closing this, because it is too much opinion based.

[timotheecour]

@krux02 do you mind re-opening this? too much opinion based is also opinion based after all

There was not enough time for feedback. Just make sure to always import both, os and ospaths is akin to saying ospaths should be merged in os, and I'm not sure we wanna go there either (at least that's a separate discussion from this one)

ospaths is meant for pure path manipulation, but for pragmatic reasons including currentDir (and as is already the case, environment variable access) makes sense so that these argulably path manipulation procs can also be included there:

• absolutePath, normalizePath, relativePath

and btw I'm happy to write the PR myself once this is accepted, and expect it'll lead to zero breakage

[krux02]

If you want to discuss this, discuss it in the chat. The issue tracker is not for discussions. But if you really really want this to be open, I make you an offer. You alone have 200 open issues in Nim right now. They can't be all that important, if you look through them and remove three of the most unimportant ones, then I can reopen this issue.

[Araq]

They can't be all that important, if you look through them and remove three of the most unimportant ones, then I can reopen this issue.

Come on.... That's just ... weird politics.

[timotheecour]

If you want to discuss this, discuss it in the chat

not everyone follows chat at all times, and not everyone is on same timezone; chat is great (and more efficient) when you want to discuss something with specific person(s) that is logged in, or when you want help, not so much when you're discussing something that other people may care about; otherwise it leads to "behind closed doors" decisions (yes, irc/gitter is searchable but anything older than 1 day is hard to search unless you know when something was discussed); at least forum is better than IRC for that.

[Araq]

It's time we merge ospaths back into os.

[timotheecour]

@Araq @dom96 back in the days, 178275f49403012ca3d774f8cadcc2836eea9508 split ospaths from os with commit msg: split os into os and ospaths parts; ospaths is available for NimScript; better NimScript support what would be implication for NimScript if ospaths were merged back into os?

independently of that, I prefer keeping ospaths separate from os (and already sent out a PR for addressing this issue), it just seems cleaner to separate out path manipulation procs (+ getCurrentDir for pragmatism) from rest of os for sake of modularity (one additional argument could be we may want to consider a future ospaths2 that replaces ospaths based on typesafe paths)

[Araq]

what would be implication for NimScript if ospaths were merged back into os?

That only some procs in os.nim are available for NimScript. They need to be documented.

independently of that, I prefer keeping ospaths separate from os (and already sent out a PR for addressing this issue), it just seems cleaner to separate out path manipulation procs (+ getCurrentDir for pragmatism)

But "+ getCurrentDir for pragmatism" means the split doesn't work well. Anyhow, ospaths is not just OS independent string processing anymore, so it's all messy, having a single os.nim feels much less messy.

[timotheecour]

well the proper fix here would be to allow cyclic dependencies (with care taken so that import os doesn't import the world, which can be enforced via tooling that computes module import graph and some checks to make sure we don't accidentally add unwanted dependencies)

a single import os also means more potential for symbol clashes as os 's scope is much larger than ospaths

[Araq]

That's not the "proper fix" that's a terrible design. And symbol clashes are a thing for import os except ... or from os import nil.

[timotheecour]

That's not the "proper fix" that's a terrible design

this should be discussed in a separate place dedicated for cyclic import, I don't want to conflate issues.

And symbol clashes are a thing for import os except ... or from os import nil.

I regret raising that point; yes, these work for symbol clashes.

But my point about modularization still stands; /cc @krux02 that's not an opinion but a fact: in pretty much all languages I've used you have an ospaths equivalent that isn't bundled with the rest of os, for reasons I already mentioned: os is much larger in scope and ospaths scope is relatively well defined:

• in D: std.path https://dlang.org/phobos/std_path.html
• in python: https://docs.python.org/3/library/os.path.html
• nodejs: https://nodejs.org/api/path.html
• go: https://golang.org/pkg/path/
• java: https://docs.oracle.com/javase/9/docs/api/java/nio/file/Path.html
• C++ boost: boost/filesystem/path.hpp
• C# : https://docs.microsoft.com/en-us/dotnet/api/system.io.path?view=netframework-4.7.2

I don't mind Nim doing things differently from every other language when there's an advantage to it, but this isn't the case here. I don't see any cons to the PR I sent out.

[Araq]

On the contrary, moving existing stuff around pointlessly has no advantage.