Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

bpo-29779: new environment variable PYTHONHISTORY.#473

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
ghost wants to merge1 commit intopython:masterfromunknown repository
Closed

bpo-29779: new environment variable PYTHONHISTORY.#473

ghost wants to merge1 commit intopython:masterfromunknown repository

Conversation

ghost
Copy link

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

@the-knights-who-say-ni

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 thePSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username onbugs.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:

  1. If you don't have an account on b.p.o, pleasecreate one
  2. Make sure your GitHub username is listed in"Your Details" at b.p.o
  3. If you have not already done so, please sign thePSF contributor agreement. The "bugs.python.org username " requested by the form is the "Login name" field under "Your Details".
  4. If you just signed the CLA, pleasewait at least one US business day and then check "Your Details" onbugs.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)
  5. Reply here saying you have completed the above steps

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

@mention-bot
Copy link

@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.

@ghost
Copy link
Author

I signed the CLA 😉

@warsaw
Copy link
Member

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

@ghost
Copy link
Author

@warsaw I added this to Misc/python.man

@warsaw
Copy link
Member

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

Copy link
Member

@warsawwarsaw left a comment

Choose a reason for hiding this comment

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

LGTM, with one very minor style nit.

Lib/site.py Outdated
history =os.path.join(os.path.expanduser('~'),
'.python_history')
history =set_history_file()

Copy link
Member

Choose a reason for hiding this comment

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

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

@ghost
Copy link
Author

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

@warsaw
Copy link
Member

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 aMisc/NEWS entry (knowing that our workflow for that kind of sucks and we might get conflicts)?

Copy link
Member

@ned-deilyned-deily left a comment

Choose a reason for hiding this comment

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

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.

@ned-deily
Copy link
Member

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.

@ghostghost changed the titleNew environment variable PYTHONHISTORY.bpo-29779: new environment variable PYTHONHISTORY.Mar 10, 2017
@ghost
Copy link
Author

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

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

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

Copy link
Member

@warsawwarsaw left a comment

Choose a reason for hiding this comment

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

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

Lib/site.py Outdated
if history_file:
return history_file
return os.path.join(os.path.expanduser('~'),
'.python_history')
Copy link
Member

Choose a reason for hiding this comment

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

You could also do something like:

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

Alsoset_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.

Misc/NEWS Outdated
@@ -10,6 +10,9 @@ What's New in Python 3.7.0 alpha 1?
Core and Builtins
-----------------

- Issue #29779: New environment variable PYTHONHISTORY if
Copy link
Member

Choose a reason for hiding this comment

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

ChangeIssue #29779 tobpo-29779. That's the new style of marking issue numbers.

@warsaw
Copy link
Member

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

@ned-deily
Copy link
Member

Thanks for addressing my comments!

@ghost
Copy link
Author

ghost commentedMar 10, 2017
edited by ghost
Loading

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

@warsaw
Copy link
Member

@0xl3vi We still need a test.

@warsaw
Copy link
Member

Oh and yay for Misc/NEWS conflicts. :(

@warsaw
Copy link
Member

Also notethis report which I can confirm.

@ghost
Copy link
Author

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

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

@warsaw
Copy link
Member

@0xl3vi At the very least, I think a test coveringgethistoryfile() 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 forenablerlcompleter() would be nice.

The bug happens when the file pointed to byPYTHONHISTORY 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.
@ghost
Copy link
Author

ghost commentedMar 15, 2017
edited by ghost
Loading

@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?

Copy link
Member

@berkerpeksagberkerpeksag left a comment

Choose a reason for hiding this comment

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

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

Thanks!

.. 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.
Copy link
Member

Choose a reason for hiding this comment

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

``~/.python_history``

@@ -713,6 +713,13 @@ conflict.

.. versionadded:: 3.6

.. envvar:: PYTHONHISTORY

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

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

What ifos.environ.get("PYTHONHISTORY") returnsNone?

@@ -259,6 +259,10 @@ def test_getsitepackages(self):
wanted = os.path.join('xoxo', 'lib', 'site-packages')
self.assertEqual(dirs[1], wanted)

def test_gethistoryfile(self):
os.environ['PYTHONHISTORY'] = "xoxo"
Copy link
Member

Choose a reason for hiding this comment

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

Please useEnvironmentVarGuard intest.support to set env variables.

@@ -10,6 +10,9 @@ What's New in Python 3.7.0 alpha 1?
Core and Builtins
-----------------

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

Choose a reason for hiding this comment

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

Please add "Patch by Your Name."

@@ -431,6 +431,10 @@ values.

The integer must be a decimal number in the range [0,4294967295]. Specifying
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.
Copy link
Member

Choose a reason for hiding this comment

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

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.

@@ -105,7 +105,8 @@ static const char usage_6[] =
" predictable seed.\n"
"PYTHONMALLOC: set the Python memory allocators and/or install debug hooks\n"
" 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";
Copy link
Member

Choose a reason for hiding this comment

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

This line is a bit long.

@ghost
Copy link
Author

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

@ghostghost closed thisMar 23, 2017
@warsaw
Copy link
Member

@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.

@ghost
Copy link
Author

@warsaw sure if you want.
thanks 👍

@ZackerySpytz
Copy link
Contributor

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

yan12125 reacted with thumbs up emoji

jaraco pushed a commit that referenced this pull requestDec 2, 2022
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>
@0xl3vi0xl3vimannequin mentioned this pull requestApr 10, 2022
This pull request wasclosed.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@the-knights-who-say-ni@mention-bot@warsaw@ned-deily@ZackerySpytz@berkerpeksag@methane

[8]ページ先頭

©2009-2025 Movatter.jp