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

Fix warning admonition error #48909

Merged
merged 2 commits into from
Mar 6, 2023
Merged

Fix warning admonition error #48909

merged 2 commits into from
Mar 6, 2023

Conversation

udohjeremiah
Copy link
Contributor

The current docstring for withenv does not print the warning-admonition as desired:

help?> withenv
search: withenv

  withenv(f, kv::Pair...)

  Execute f in an environment that is temporarily modified (not replaced as
  in setenv) by zero or more "var"=>val arguments kv. withenv is generally
  used via the withenv(kv...) do ... end syntax. A value of nothing can be
  used to temporarily unset an environment variable (if it is set). When
  withenv returns, the original environment has been restored.

  !!! warning
  Changing the environment is not thread-safe. For running external commands with a different
  environment from the parent process, prefer using [`addenv`](@ref) over `withenv`.

The waring-admonition should be formatted this way for it to be printed as desired:

  !!! warning
      Changing the environment is not thread-safe. For running external commands with a different
      environment from the parent process, prefer using [`addenv`](@ref) over `withenv`.

@udohjeremiah udohjeremiah added the docs This change adds or pertains to documentation label Mar 6, 2023
Copy link
Member

@inkydragon inkydragon left a comment

Choose a reason for hiding this comment

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

Also:

julia/base/env.jl

Lines 77 to 78 in 7eb9615

!!! warning
Mutating the environment is not thread-safe.

Copy link
Member

@inkydragon inkydragon left a comment

Choose a reason for hiding this comment

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

LGTM

@inkydragon inkydragon added the merge me PR is reviewed. Merge when all tests are passing label Mar 6, 2023
@fredrikekre fredrikekre merged commit a70bbdf into JuliaLang:master Mar 6, 2023
@oscardssmith oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label Mar 6, 2023
@udohjeremiah udohjeremiah deleted the patch-1 branch March 6, 2023 17:03
@imciner2 imciner2 mentioned this pull request Aug 11, 2023
35 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants