-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[std/os] split and re-export #20593
[std/os] split and re-export #20593
Conversation
You need to rebase. |
from std/envvars import getEnv, existsEnv, delEnv, putEnv, envPairs | ||
from std/os import dirExists, fileExists, walkDir, getAppFilename | ||
from std/os import walkDir, getAppFilename, dirExists, fileExists |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be changed to from std/private/oscommon import dirExists, fileExists
in the following PRs because of bootstrapping.
fwiw, this is an unrelated CI failure /home/runner/.nimble/pkgs/measuremancer-0.1.1/measuremancer.nim(280, 15) Error: ambiguous call; both measuremancer.*(m: Measurement[*.U], x: T: FloatLike) [proc declared in /home/runner/.nimble/pkgs/measuremancer-0.1.1/measuremancer.nim(262, 6)] and measuremancer.*(m: Measurement[*.T], x: T: FloatLike) [proc declared in /home/runner/.nimble/pkgs/measuremancer-0.1.1/measuremancer.nim(265, 6)] match for: (T, float)
stack trace: (most recent call last) |
Oh, I had forgotten that |
I've written some tests, set up a CI for Aside from that, maybe |
Sounds good, it would be better if it is on the nimble packages indexes. |
Oh, I didn't realize I never added it. Will do. edit: here nim-lang/packages#2392 |
## * `removeFile proc`_ | ||
## * `moveFile proc`_ | ||
|
||
doAssert card(copyFlagSymlink * options) == 1, "There should be exactly " & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing but terrible API design! Instead of options: set[CopyFlag]
it should have been option: CopyFlag
!
|
||
proc copyFileWithPermissions*(source, dest: string, | ||
ignorePermissionErrors = true, | ||
options = {cfSymlinkFollow}) {.noWeirdTarget.} = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has the same design bug.
|
||
if not tryMoveFSObject(source, dest, isDir = false): | ||
when defined(windows): | ||
doAssert false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should have been raiseOSError
or similar.
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
* [std/os] split and export * move to private modules * fixes docs and tests Co-authored-by: xflywind <[email protected]>
The PR intends to import/reexport
std/private/osfiles
,std/private/ospaths2
,std/private/osdirs
,std/private/ossymlinks
forstd/os
module. #20582 should be a simple wrapper around these modules based on type safe definitions.The APIs for
std/paths
,std/files
,std/dirs
andstd/symlinks
will be refined based on these components. There are not necessarily identical.