Skip to content

Fix #384 #395

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Sep 26, 2014
Merged

Fix #384 #395

merged 13 commits into from
Sep 26, 2014

Conversation

lohmataja
Copy link
Contributor

show_source now shows the following error messages:

  • for empty string: ""Cannot get source of an empty string"
  • for a string that matches a built-in: "Cannot access source of "
  • for a string that doesn't match any object: "Cannot get source: X is not defined"
  • if the object doesn't have source code: "No source code found for X"
  • else: the error message from the original inspect.getsource() error

lohmataja and others added 12 commits September 24, 2014 13:59
@lohmataja
Fix get_source_of_current_name to raise exceptions instead of returni…
c0bdca0 
…ng None
@lohmataja
Fix p_key to catch exceptions from get_source_of_current_name
d4dda9b
@lohmataja
Fix docstring for get_source_of_current_name to match the new behavior
a4552db
@lohmataja @mlauter
Fix get_source_of_current_name to raise exceptions instead of returni…
f17eeec 
…ng None
@lohmataja @mlauter
Fix p_key to catch exceptions from get_source_of_current_name
75e8e35
@lohmataja @mlauter
Fix docstring for get_source_of_current_name to match the new behavior
ca74c46
@mlauter
fix show_source method of curtsiesfrontend/repl to catch exceptions f…
13eb518 
…rom get_source_of_current_name
@mlauter
modify the NameError message displayed by show_source
31db4c7
@mlauter
send long error messages originating in inspect.getsource to pager no…
1c218a1 
…t status_bar
@mlauter
revert to shortening messages thrown by inspect.getsource before raising
dab1ccb
@mlauter
simplify error message raising and handling
73d74b2
@lohmataja
Merge all changes in fix_384
5b1143e
@thomasballinger
Copy link
Member

This is great!

  • Special-casing builtins is brilliant - those are the first things you try to look up the source for, and when they don't work it's confusing.
  • I think "Cannot get source of an empty string" is confusing to the user - they didn't try to get the source for >>> "", then just tried it on an empty line. The fact that we represent the current line of the REPL as a string isn't relevant IMO.
  • having different messages for things with source code vs things that don't have it is great

@lohmataja
Copy link
Contributor Author

So, for the empty string, any suggestions? Maybe smth like "Type a function name to view its source code"?

@thomasballinger
Copy link
Member

Sure, that's good, or "nothing to get source of" or similar. The message you had was good, I just thought the phrase "empty string" could be confusing.

@lohmataja
Copy link
Contributor Author

Ok, done. Just pushed an update, now:

  • Error message on empty string: "Nothing to get source of"

thomasballinger added a commit that referenced this pull request Sep 26, 2014
@thomasballinger
Merge pull request #395 from lohmataja/fix_384
ffa49f0 
Fix #384
@thomasballinger thomasballinger merged commit ffa49f0 into bpython:master Sep 26, 2014
@thomasballinger
Copy link
Member

Looks good, thanks!

Sign up for free to 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.
4 participants
@lohmataja @thomasballinger @Trundle @mlauter