| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -2614,18 +2614,50 @@ def browse(port=0, *, open_browser=True, hostname='localhost'): | |
| def ispath(x): | ||
| return isinstance(x, str) and x.find(os.sep) >= 0 | ||
|
|
||
| def _get_revised_path(given_path, argv0): | ||
| """Ensures current directory is on returned path, and argv0 directory is not | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityMissing a period.
Sorry, something went wrong.
All reactions
|
||
|
|
||
| Exception: argv0 dir is left alone if it's also pydoc's directory. | ||
|
|
||
| Returns a new path entry list, or None if no adjustment is needed. | ||
| """ | ||
| # Scripts may get the current directory in their path by default if they're | ||
| # run with the -m switch, or directly from the current directory. | ||
| # The interactive prompt also allows imports from the current directory. | ||
|
|
||
| # Accordingly, if the current directory is already present, don't make | ||
| # any changes to the given_path | ||
| if '' in given_path or os.curdir in given_path or os.getcwd() in given_path: | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIn what circumstances sys.path contains '' or '.'?
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality'' will be present for -c and the interactive prompt, '.' will only be present if you added it explicitly (originally it was added by pydoc.cli() itself, but I changed that as part of this PR)
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityWhat if just remove the whole sys.path patching code? Both ./python -m pydoc and ./python Lib/pydoc.py looks working after this.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThe problem the path manipulation solves is that pydoc itself needs to be able to find modules in the current directory (where the user's own code is likely to be), even when run via a wrapper script installed as /usr/bin/pydoc (or the Windows equivalent). I'm not sure there's a regression test for that, though.
Sorry, something went wrong.
All reactions
|
||
| return None | ||
|
|
||
| # Otherwise, add the current directory to the given path, and remove the | ||
| # script directory (as long as the latter isn't also pydoc's directory. | ||
| stdlib_dir = os.path.dirname(__file__) | ||
| script_dir = os.path.dirname(argv0) | ||
| revised_path = given_path.copy() | ||
| if script_dir in given_path and not os.path.samefile(script_dir, stdlib_dir): | ||
| revised_path.remove(script_dir) | ||
| revised_path.insert(0, os.getcwd()) | ||
| return revised_path | ||
|
|
||
|
|
||
| # Note: the tests only cover _get_revised_path, not _adjust_cli_path itself | ||
| def _adjust_cli_sys_path(): | ||
| """Ensures current directory is on sys.path, and __main__ directory is not | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityMissing a period.
Sorry, something went wrong.
All reactions
|
||
|
|
||
| Exception: __main__ dir is left alone if it's also pydoc's directory. | ||
| """ | ||
| revised_path = _get_revised_path(sys.path, sys.argv[0]) | ||
| if revised_path is not None: | ||
| sys.path[:] = revised_path | ||
|
|
||
|
|
||
| def cli(): | ||
| """Command-line interface (looks at sys.argv to decide what to do).""" | ||
| import getopt | ||
| class BadUsage(Exception): pass | ||
|
|
||
| # Scripts don't get the current directory in their path by default | ||
| # unless they are run with the '-m' switch | ||
| if '' not in sys.path: | ||
| scriptdir = os.path.dirname(sys.argv[0]) | ||
| if scriptdir in sys.path: | ||
| sys.path.remove(scriptdir) | ||
| sys.path.insert(0, '.') | ||
| _adjust_cli_sys_path() | ||
|
|
||
| try: | ||
| opts, args = getopt.getopt(sys.argv[1:], 'bk:n:p:w') | ||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -11,6 +11,7 @@ | |
| import re | ||
| import stat | ||
| import string | ||
| import tempfile | ||
| import test.support | ||
| import time | ||
| import types | ||
| Expand Down Expand Up | @@ -1084,6 +1085,71 @@ def test_resolve_false(self): | |
| self.assertIn('class Enum', helptext) | ||
|
|
||
|
|
||
| class TestInternalUtilities(unittest.TestCase): | ||
|
|
||
| def setUp(self): | ||
| tmpdir = tempfile.TemporaryDirectory() | ||
| self.argv0dir = tmpdir.name | ||
| self.argv0 = os.path.join(tmpdir.name, "nonexistent") | ||
| self.addCleanup(tmpdir.cleanup) | ||
| self.abs_curdir = abs_curdir = os.getcwd() | ||
| self.curdir_spellings = ["", os.curdir, abs_curdir] | ||
|
|
||
| def _get_revised_path(self, given_path, argv0=None): | ||
| # Checking that pydoc.cli() actually calls pydoc._get_revised_path() | ||
| # is handled via code review (at least for now). | ||
| if argv0 is None: | ||
| argv0 = self.argv0 | ||
| return pydoc._get_revised_path(given_path, argv0) | ||
|
|
||
| def _get_starting_path(self): | ||
| # Get a copy of sys.path without the current directory | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityMissing a period.
Sorry, something went wrong.
All reactions
|
||
| clean_path = sys.path.copy() | ||
| for spelling in self.curdir_spellings: | ||
| for __ in range(clean_path.count(spelling)): | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualitySingle _.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityUsing double-underscore as my throwaway variable name is a habit for me these days: https://stackoverflow.com/a/5893946/597742
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIt is hard to distinguish __ from _ or ___. And since names starting with underscores have special meaning, it looks as a false signal that there is is something special here. I would use i here.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI actually went to PEP 8 to see if it stated a preference for throwaway variable names, but it's currently silent on the topic.
Sorry, something went wrong.
All reactions
|
||
| clean_path.remove(spelling) | ||
| return clean_path | ||
|
|
||
| def test_sys_path_adjustment_adds_missing_curdir(self): | ||
| clean_path = self._get_starting_path() | ||
| expected_path = [self.abs_curdir] + clean_path | ||
| self.assertEqual(self._get_revised_path(clean_path), expected_path) | ||
|
|
||
| def test_sys_path_adjustment_removes_argv0_dir(self): | ||
| clean_path = self._get_starting_path() | ||
| expected_path = [self.abs_curdir] + clean_path | ||
| leading_argv0dir = [self.argv0dir] + clean_path | ||
| self.assertEqual(self._get_revised_path(leading_argv0dir), expected_path) | ||
| trailing_argv0dir = clean_path + [self.argv0dir] | ||
| self.assertEqual(self._get_revised_path(trailing_argv0dir), expected_path) | ||
|
|
||
|
|
||
| def test_sys_path_adjustment_protects_pydoc_dir(self): | ||
| def _get_revised_path(given_path): | ||
| return self._get_revised_path(given_path, argv0=pydoc.__file__) | ||
| clean_path = self._get_starting_path() | ||
| leading_argv0dir = [self.argv0dir] + clean_path | ||
| expected_path = [self.abs_curdir] + leading_argv0dir | ||
| self.assertEqual(_get_revised_path(leading_argv0dir), expected_path) | ||
| trailing_argv0dir = clean_path + [self.argv0dir] | ||
| expected_path = [self.abs_curdir] + trailing_argv0dir | ||
| self.assertEqual(_get_revised_path(trailing_argv0dir), expected_path) | ||
|
|
||
| def test_sys_path_adjustment_when_curdir_already_included(self): | ||
| clean_path = self._get_starting_path() | ||
| for spelling in self.curdir_spellings: | ||
| with self.subTest(curdir_spelling=spelling): | ||
| # If curdir is already present, no alterations are made at all | ||
| leading_curdir = [spelling] + clean_path | ||
| self.assertIsNone(self._get_revised_path(leading_curdir)) | ||
| trailing_curdir = clean_path + [spelling] | ||
| self.assertIsNone(self._get_revised_path(trailing_curdir)) | ||
| leading_argv0dir = [self.argv0dir] + leading_curdir | ||
| self.assertIsNone(self._get_revised_path(leading_argv0dir)) | ||
| trailing_argv0dir = trailing_curdir + [self.argv0dir] | ||
| self.assertIsNone(self._get_revised_path(trailing_argv0dir)) | ||
|
|
||
|
|
||
| @reap_threads | ||
| def test_main(): | ||
| try: | ||
| Expand All | @@ -1094,6 +1160,7 @@ def test_main(): | |
| PydocUrlHandlerTest, | ||
| TestHelper, | ||
| PydocWithMetaClasses, | ||
| TestInternalUtilities, | ||
| ) | ||
| finally: | ||
| reap_children() | ||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| Fixed regression when running pydoc with the ``-m`` switch. (The regression | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality:option:`-m`?
Sorry, something went wrong.
All reactions
|
||
| was introduced in 3.7.0b3 by the resolution of bpo-33053) | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityMissed period. Shouldn't the reference to other issue be written as :issue:`33053`?
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityBleh, this is the main downside of the switch to separate news snippets - no other surrounding entries to prompt styling fixes.
Sorry, something went wrong.
All reactions
|
||
|
|
||
| This fix also changed pydoc to add ``os.getcwd()`` to ``sys.path`` when | ||
| necessary, rather than adding ``"."``. | ||
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualityos.getcwd() or the starting directory?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityDepends on why you're looking for the empty string (in the case of pydoc for example, it knows it hasn't changed the working directory, so the starting directory and the current directory are the same thing).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityBy "the current working directory" do you mean the empty string, not os.getcwd()?
The wording here looks slightly confusing.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityYeah, I tried to make it clearer in the last set of updates, but it's hard to concisely explain the difference between "current working directory at application startup" and "current working directory at time of import resolution".
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.