bpo-29779: new environment variable PYTHONHISTORY. · Pull Request #473 · python/cpython · GitHub

if this variable is set the user can change the location of
a ~/.python_history file without adding hook to PYTHONSTARTUP.

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

If you don't have an account on b.p.o, please create one
Make sure your GitHub username is listed in "Your Details" at b.p.o
If you have not already done so, please sign the PSF contributor agreement. The "bugs.python.org username " requested by the form is the "Login name" field under "Your Details".
If you just signed the CLA, please wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

@0xl3vi, thanks for your PR! By analyzing the history of the files in this pull request, we identified @brettcannon, @tiran, @vsajip, @doko42 and @freddrake to be potential reviewers.

I signed the CLA 😉

This is a nice change, and it would probably let me get rid of my $PYTHONSTARTUP setting. I wonder if python3 -h should mention this new envar?

@warsaw I added this to Misc/python.man

@0xl3vi Thanks, that looks good. It's still not mentioned in python3 -h but I'm not entirely sure it needs to. There are other PYTHON* environment variables not mentioned in the --help output.

LGTM, with one very minor style nit.

Minor style nit: You don't need this extra blank line.

@warsaw I updated main.c for python -h,
blank line fixed.

The changes look great to me, but I'm going to wait just a bit before merging this.

I'd like to see if any other core dev has an opinion about the change
The AppVeyor test is failing, although I suspect that is unrelated to your change

I don't have a Windows environment to test that on. Maybe @zooba can weigh in on that. Alternatively, try rebasing against master if it's been fixed there.

If there are no other follow ups by this weekend, I'll merge your change.

Can you please also add a Misc/NEWS entry (knowing that our workflow for that kind of sucks and we might get conflicts)?

Environment variables are also documented in the Python Setup and Usage document (https://docs.python.org/3/using/cmdline.html#environment-variables) which is generated from Doc/using/cmdline.rst.

Also there should be an issue opened on bugs.python.org to document this and the issue number added to the title of this pull request.

@warsaw I tried rebasing against master and the build passed!
and Misc/NEWS updated.

@ned-deily I updated cmdline.rst and created new issue http://bugs.python.org/issue29779

P.S: I need to add bpo-XXX or just the issue number in bugs.python.org?

Just a couple of other thoughts on the implementation and NEWS file entry.

You could also do something like:

    return os.environ.get(
        'PYTHONHISTORY',
        os.path.expanduser(os.path.join('~', '.python_history')))

Also set_history_file() is a little bit of a misnomer since it's not actually setting it, but calculating it instead. OTOH, I bet the more compact form could just be inlined below.

Change Issue #29779 to bpo-29779. That's the new style of marking issue numbers.

Oh, and it would be nice to add a test for codecov.

Thanks for addressing my comments!

@warsaw all done, I changed set_history_file() to gethistoryfile().
and added test.

@0xl3vi We still need a test.

Oh and yay for Misc/NEWS conflicts. :(

Also note this report which I can confirm.

@warsaw sorry for the dealy.
what tests we need add?

regarding this
If the PYTHONHISTORY is empty it will fall back to .python_history.
there is no error on my build

@0xl3vi At the very least, I think a test covering gethistoryfile() would be useful. If you can mock out enough of the readline stuff (or other safeguards so as not to interfere with the environment of a person running the tests), then adding some coverage for enablerlcompleter() would be nice.

The bug happens when the file pointed to by PYTHONHISTORY does not exist.

this patch adds new environment variable PYTHONHISTORY, with this you can change the location of a python_history file, without using readline hook in PYTHONSTARTUP. In case PYTHONHISTORY will be empty or not set, it wil fall back to the default history file.

@warsaw OK updated.

In case PYTHONHISTORY will be empty or not set,
it wil fall back to the default history file.
if the file pointed by PYTHONHISTORY does not exist readline will create one.

The test checks if gethistoryfile() reads the variable.
Bash will expand '~' so we dont need to to test the path.

what do you think?

This also needs to be documented in Doc/whatsnew/3.7.rst (and please add "(Contributed by Your Name.)")

Thanks!

``~/.python_history``

"a ``.python_history`` file" or just "a history file".

What if os.environ.get("PYTHONHISTORY") returns None?

Please use EnvironmentVarGuard in test.support to set env variables.

Please add "Patch by Your Name."

File names should be italic in man pages so \fI~/.python_history\fI or

python_history file, by default it will be
.IR ~/.python_history .

You may need to escape ~ or / characters.

This line is a bit long.

OK, I don't have time to do it.
If someone else want to implement this please do so.

@0xl3vi Hi. I'm sorry this has taken so long. Are you saying in your comment that you don't have time to finish this branch? If so, that's okay and we greatly appreciate your contribution so far. I'm willing to finish this branch for you if you'd like.

@warsaw sure if you want.
thanks 👍

@warsaw @berkerpeksag I have created a new PR for this feature: GH-13208. It addresses all of @berkerpeksag's comments.

Bumps [sentry-sdk](https://github.com/getsentry/sentry-python) from 1.1.0 to 1.3.1. - [Release notes](https://github.com/getsentry/sentry-python/releases) - [Changelog](https://github.com/getsentry/sentry-python/blob/master/CHANGELOG.md) - [Commits](getsentry/sentry-python@1.1.0...1.3.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Łukasz Langa <lukasz@langa.pl>

the-knights-who-say-ni added the CLA not signed label Mar 5, 2017

methane removed the CLA not signed label Mar 7, 2017

the-knights-who-say-ni added the CLA signed label Mar 7, 2017

warsaw approved these changes Mar 8, 2017

View reviewed changes

ned-deily requested changes Mar 9, 2017

View reviewed changes

ghost changed the title New environment variable PYTHONHISTORY. bpo-29779: new environment variable PYTHONHISTORY. Mar 10, 2017

warsaw requested changes Mar 10, 2017

View reviewed changes

ned-deily approved these changes Mar 10, 2017

View reviewed changes

berkerpeksag requested changes Mar 22, 2017

View reviewed changes

ghost closed this Mar 23, 2017

0xl3vi mannequin mentioned this pull request Apr 10, 2022

New environment variable PYTHON_HISTORY #73965

Closed

This pull request was closed.

+                   if not use the default ~/.python_history file.
+                   """
+                  h = os.environ.get("PYTHONHISTORY")
+                  if h != '':

               the value 0 will disable hash randomization.
+              .IP PYTHONHISTORY
+              If this is set you can change the location of a
+              python_history file, by default it will be ~/.python_history.

               "   on Python memory allocators. Use PYTHONMALLOC=debug to install debug\n"
-              "   hooks.\n";
+              "   hooks.\n"
+              "PYTHONHISTORY: If this is set, you can change the location of a python_history file.\n";

+              .. envvar:: PYTHONHISTORY
+                 If set to a non-empty string, you can change the location of a python_history
+                 file, by default it will be in ~/.python_history.


		.. envvar:: PYTHONHISTORY

		If set to a non-empty string, you can change the location of a python_history

                           self.assertEqual(dirs[1], wanted)
+                  def test_gethistoryfile(self):
+                      os.environ['PYTHONHISTORY'] = "xoxo"

               -----------------
+              - bpo-29779: New environment variable PYTHONHISTORY if
+                this is set you can change the location of a python_history file.

bpo-29779: new environment variable PYTHONHISTORY. · Pull Request #473 · python/cpython · GitHub

Uh oh!

Uh oh!

bpo-29779: new environment variable PYTHONHISTORY. - #473

bpo-29779: new environment variable PYTHONHISTORY.#473ghost wants to merge 1 commit intomasterpython/cpython:masterfrom unknown repositoryCopy head branch name to clipboard

Conversation

ghost commented Mar 5, 2017

Uh oh!

the-knights-who-say-ni commented Mar 5, 2017

Uh oh!

mention-bot commented Mar 5, 2017

Uh oh!

ghost commented Mar 5, 2017

Uh oh!

warsaw commented Mar 5, 2017

Uh oh!

ghost commented Mar 8, 2017

Uh oh!

warsaw commented Mar 8, 2017

Uh oh!

warsaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Mar 9, 2017

Uh oh!

warsaw commented Mar 9, 2017

Uh oh!

ned-deily left a comment

Choose a reason for hiding this comment

Uh oh!

ned-deily commented Mar 9, 2017

Uh oh!

ghost commented Mar 10, 2017

Uh oh!

warsaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warsaw commented Mar 10, 2017

Uh oh!

ned-deily commented Mar 10, 2017

Uh oh!

ghost commented Mar 10, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

warsaw commented Mar 14, 2017

Uh oh!

warsaw commented Mar 14, 2017

Uh oh!

warsaw commented Mar 14, 2017

Uh oh!

ghost commented Mar 15, 2017

Uh oh!

warsaw commented Mar 15, 2017

Uh oh!

ghost commented Mar 15, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

berkerpeksag left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bpo-29779: new environment variable PYTHONHISTORY.#473
ghost wants to merge 1 commit into
masterfrom
unknown repository

ghost commented Mar 10, 2017 •
edited by ghost
Loading

ghost commented Mar 15, 2017 •
edited by ghost
Loading