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

[superseded] partially address issue #8217 by running user config.nims if possible #8233

Closed
wants to merge 1 commit into from

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Jul 7, 2018

/cc @Araq

this PR runs user's config.nims if it exists and another nims hasn't also ran before, allowing to apply user configuration globally via nimscript instead of the more limited language nim.cfg

ideally, we would instead do the following:

  • run system config.nims if exists
  • run user config.nims if exists (eg in posix: ~/.config/config.nims)
  • run project config.nims if exists
  • run project provided_file.nims if if exists and nim cmd was run on provided_file.nim

but that logic fails because call runNimScript multiple times in errors with: ???(0, 0) Error: internal error: n is not nil
(see #8235)
(EDIT: FIXED)

TODO in future PR: apply that same logic to nimsuggest.nim (and refactor to remove code duplication between nimsuggest.nim and nim.nim)

@@ -581,6 +581,7 @@ proc processCmdLine*(pass: TCmdLinePass, cmd: string; conf: ConfigRef) =
conf.projectName = a
# if processArgument(pass, p, argsCount): break

# TODO: refactor with nimconf.nim to avoid duplicating complex logic
Copy link
Member

@Araq Araq Jul 16, 2018

Choose a reason for hiding this comment

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

Good catch. But I cannot merge a PR that makes things worse, then nimsuggest disagrees with nim's way of reading the configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, pending refactoring via OOP here #8682 (comment)

@timotheecour timotheecour changed the title partially address issue #8217 by running user config.nims if possible [overridden] partially address issue #8217 by running user config.nims if possible Aug 29, 2018
@timotheecour timotheecour changed the title [overridden] partially address issue #8217 by running user config.nims if possible [superseded] partially address issue #8217 by running user config.nims if possible Aug 29, 2018
@timotheecour timotheecour deleted the pr_config_nims branch September 3, 2018 05:44
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.

2 participants