-
-
Notifications
You must be signed in to change notification settings - Fork 680
improve the error message when require and provide transformers are used outside of require
and provide
#5044
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
base: master
Are you sure you want to change the base?
Conversation
This looks ok to me. I always find "must be used in X" messages tricky, because other macros might expand to X. Maybe "a require form" and "a provide form" here (where "require" and "provide" are still not code-quoted, the same as here). Although something could rely on distinguishing values that it controls with |
"is a provide transformer, and cannot be used in this context"? Off-topic, but do we really want to keep calling these require/provide transformers? Other forms would call them "expanders". |
Don't use implementations names for error messages. (I know that Americans call it a "cell phone" to honor the implementation technique but it is a mobile phone as opposed to a "wall phone" (which is what kids call the old things). But I don't think that' good practice.) Perhaps something like this "must be used to specify a contract on an import [from another module] and not by itself" and we could omit the bracketed words. |
This change isn't just about contracts, it turns out that many of the things that go inside a
produces
and similarly for I see that the documentation uses the term As for:
Given that the only way to signal these errors is by making the value be a procedure, it seems like that'd be an unfortunate form of backward compatibility to have to honor. But I agree with you that it is unlikely. |
Or how about:
|
…sed outside of `require` and `provide`
09b7434
to
c334ee8
Compare
With the push I just did, this program:
(in the file tmp2.rkt) produces this message:
but I'm not sure I've got the quoting conventions right. I have a memory that the quotes should be |
I think this is an improvement. (I thought Matthew's policy for regular racket was to not make suggestions, such as "valid positions include the argument to |
Ah, right, I remember something like that. Unfortunately, searching in the docs for "require-spec" doesn't turn up anything useful. But perhaps this is a separate problem, where the non-terminals in blueboxes need better support in the documentation (I often miss the ability to from a reference of one of those to its definition.)
Do you mind offering an edit to the error message to fix that? (I can put it into the PR and I'll delete the part after the semicolon at the same time.) |
How about just saying “module imports (for example `require`)” — this keeps it at a level that people tend to understand and has the keyword there too?
… On Jul 23, 2024, at 10:41 AM, Robby Findler ***@***.***> wrote:
I think this is an improvement.
(I thought Matthew's policy for regular racket was to not make suggestions, such as "valid positions include the argument to require".
Ah, right, I remember something like that. Unfortunately, searching in the docs for "require-spec" doesn't turn up anything useful. But perhaps this is a separate problem, where the non-terminals in blueboxes need better support in the documentation (I often miss the ability to from a reference of one of those to its definition.)
My personal preference is to not refer to subforms such as the inside of require as "argument".)
Do you mind offering an edit to the error message to fix that? (I can put it into the PR and I'll delete the part after the semicolon at the same time.)
—
Reply to this email directly, view it on GitHub <#5044 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAL7T66IK42JGCDAB46COADZNZTQJAVCNFSM6AAAAABLERGSVWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBVGQ2DONJYGQ>.
You are receiving this because you commented.
|
Some opinions on:
Here's another concrete suggestion to refine the message:
@sorawee: A require or provide transformer is not an "expander" in the same sense as many other contexts, because its input and output are not in the same kind of thing. That's why I went with "transformer" originally. Granted, "provide pre-transformer" is pretty awkward terminology for something that does turn out to be an expander, but it would be even more confusing to have "provide expander" and "provide transformer" as different things. That's the rationale that got us here, anyway. Quoting conventions: use backquotes instead of backquote-forwardquote. There may be lingering backquote-forwardquotes, but I've been trying to convert them all. |
This error message improvement is in response this post on discourse.