Skip to content
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

Give warnings when using options not supported by a Builtin #645

Merged
merged 4 commits into from
Sep 17, 2020

Conversation

poke1024
Copy link
Contributor

@poke1024 poke1024 commented Nov 3, 2016

Currently, if options are given for commands that are not supported, they are simply ignored. This can be very confusing for novice users, especially if they mistype an option (e.g. FieldSeperetors instead of FieldSeparators).

This PR adds a new meta option called $OptionSyntax that may be either Strict, Warn or Ignore, and defines what should happen, if the user specifies an option that does not show up in the Builtin's options dictionary.

The default (also used if no $OptionSyntax is specified) is to give a warning but continue with evaluation. Strict gives a warning and stops the evaluation (this happens for Plot). Ignore is the old behavior.

In addition to this, this PR adds custom checks for Import and Export that gives warnings for options that are not supported by the import/export plugin.

In this current form, Import, Export and other commands will warn about unused options, which might differ from MMA. Still, I think these kinds of warnings are so useful that we should accept their general existence in Mathics. In addition, some commands, like Plot, really want to abort evaluation, if non-supported options are given, and this PR offers a solution for not having to code this checks for each single Builtin that wants to behave in that way.

@sn6uv
Copy link
Member

sn6uv commented Nov 3, 2016

This sounds like a great idea. It hits a pain point I've hit before which is trying to use an MMA option that isn't implemented in Mathics and not knowing what's gone wrong.

@GarkGarcia
Copy link
Contributor

Hey @poke1024, would you mind rebasing this too? 😅️

@GarkGarcia GarkGarcia added the needs rabase An old PR that needs to be rebased label Sep 3, 2020
GarkGarcia added a commit that referenced this pull request Sep 17, 2020
Give warnings when using options not supported by a Builtin (#645 rebase)
@GarkGarcia GarkGarcia merged commit 818794e into mathics:master Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs rabase An old PR that needs to be rebased
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants