-
Notifications
You must be signed in to change notification settings - Fork 58
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
Consider adding a --platform (or -p) option to generate #256
Comments
What do you mean by "built in"? All platforms in |
I think he means We've talked about this a few times in IRC, but I believe we both agree that it's "an open question" on how to use the @RobertBaruch Look at blinky.py in |
Here's what I ended up doing in order to (in the more general sense) get arguments added to the parser: from nmigen.cli import main_parser, main_runner
...
if __name__ == "__main__":
parser = main_parser()
parser.add_argument("--cover", action="store_true")
parser.add_argument("--bmc", action="store_true")
parser.add_argument("--insn")
# if I wanted a platform arg here, I'd add that too
args = parser.parse_args()
...
main_runner(parser, args, m)
# main(m) |
Sure. But I don't understand how would |
I could work around the lack of a --platform option like this: from nmigen.cli import main_parser, main_runner
...
if __name__ == "__main__":
parser = main_parser()
parser.add_argument("--platform")
args = parser.parse_args()
...
main_runner(parser, args, m, platform=args.platform)
# Cannot use 'main' because it doesn't support a --platform cli flag.
# main(m) Then I could just do something like |
That's the intended use of |
Ah, ok, it didn't look like those functions were intended to be exported, since they weren't in |
In general, public names (those not starting with
What would that populate it with? |
You could use |
Sure. But using a string as a platform is atypical, and in the long run I expect virtually no nMigen code will use it (even though it'll remain permitted). For example, anything that uses Using The only reason |
Makes sense. I didn't realize |
Currently you have to specify platform as a kwarg to main() and write your own arg parser to get a platform from the command line. Since platform seems to be built in to nMigen, consider being able to set it from the command line as standard.
The text was updated successfully, but these errors were encountered: