Skip to content
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

add --clearNimblePath; fixes #12601 #12609

Merged
merged 1 commit into from Nov 6, 2019
Merged

add --clearNimblePath; fixes #12601 #12609

merged 1 commit into from Nov 6, 2019

Conversation

disruptek
Copy link
Contributor

Per title; resets the list of Nimble search paths so that they may subsequently be manipulated by further nimblePath options.

@@ -102,6 +102,7 @@ Advanced options:
--putenv:key=value set an environment variable
--NimblePath:PATH add a path for Nimble support
--noNimblePath deactivate the Nimble path
--clearNimblePath empty the list of Nimble package search paths
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
--clearNimblePath empty the list of Nimble package search paths
--clearNimblePath empty the list of Nimble package search paths; more Nimble paths can be added after clearing using --NimblePath:PATH

@@ -546,6 +546,9 @@ proc disableNimblePath*(conf: ConfigRef) =
incl conf.globalOptions, optNoNimblePath
conf.lazyPaths.setLen(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the order of disableNimblePath and clearNimblePath be switched and have disableNimblePath call the other to do that setLen(0)?

@kaushalmodi
Copy link
Contributor

How do we write tests for these combinations:

Switches Effective Nimble paths
--noNimblePath none
--NimblePath:abc Default ~/.nimble + abc
--clearNimblePath --NimblePath:abc abc
--NimblePath:abc --clearNimblePath none
--NimblePath:abc --clearNimblePath --NimblePath:def def
--NimblePath:abc --clearNimblePath --NimblePath:def --noNimblePath none
--noNimblePath --NimblePath:abc --clearNimblePath --NimblePath:def none

@genotrance
Copy link
Contributor

I've not reviewed this change but am not a fan in principal since we have a goal of reducing Nim's awareness of Nimble in nim-lang/nimble#654. This adds even more behavior into Nim which will be harder to reduce or eliminate in a backwards compatible way.

@Araq
Copy link
Member

Araq commented Nov 6, 2019

@genotrance true but in the meantime people need a solution. However, the same behaviour could be added to the existing --noNimblePath.

@genotrance
Copy link
Contributor

This was discussed and --clearNimblePath persisted over modifying existing behavior which exists for a legitimate use case.

@Araq Araq merged commit 738c957 into nim-lang:devel Nov 6, 2019
kiyolee pushed a commit to kiyolee/nim that referenced this pull request Nov 7, 2019
narimiran pushed a commit to narimiran/Nim that referenced this pull request Nov 20, 2019
@disruptek disruptek deleted the nimpath branch September 12, 2020 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants