-
Notifications
You must be signed in to change notification settings - Fork 372
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 missing import in doc
macro expansion
#1679
Conversation
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.
Don't forget to update NEWS.
I didn't realize we had the |
I think this bug was introduced by the use of Also, I was thinking of putting in a PR that rewrites the |
Oh, okay, there's no need for a NEWS item if this bug was a new regression.
That sounds fine to me. |
It should print the documentation of both or allow one to specify/choose. |
@brandonwillard see the discussion in #1415 for why that's a bad idea. |
I was looking over #1416 yesterday; it's next on my list. I've been doing all this recent work in preparation for larger macro-oriented changes, so I'll need your input on that later, as well as #1407 and #277. Using Otherwise, the discussion in #1415 does not make a clear case for why a type-narrowing option — or something similar — is objectively "bad". It looks like a matter of preference, especially since the current implementation only separates the cases based on a syntax mnemonic (instead of, for instance, a naming scheme like I don't really have a preference either way, but I do think there's value in having the ability to obtain all the information in one place. |
This is moot if we can use But it is a serious usability concern, not just a personal preference. The function you use when you've forgotten how to use things should be easy to remember how to use. If you have to switch to your browser and google how to do it, what's the point of having docs in your repl at all? Just google the macro or whatever. If it's not simple, we may as well not have it. That means a single-argument form with a simple name. It's already bad that I made a There may be other approaches besides the one I chose that would meet this requirement ("something similar"? I can think of a few), but making things complicated is not it. Printing out all three is less bad, but still problematic when the docs are long.
Sure, feel free to open new issues on that point. Also see #1019, and #1044, which are sort of related. |
I don't think I understand what you want. If there's a function, macro, and tag macro with the same name, and you don't want |
Fixing If we put macros in a Tag macros might be in a similar |
9566da0
to
96f99c2
Compare
The doc macro expands without importing
importlib.import_module
, so, if the expansion namespace doesn't haveimport_module
present,doc
will fail. This PR corrects that and introduces a missing test for thedoc
macro.