-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add pattern names in SyntaxInformation and missing usage, syntax information and syntax autocompletion #603
Conversation
@daneelsan, is there any reason not to merge this now while we try to figure out how to do autocompletion for options? |
@maxitg I missed the last changes you made. I'll review it now. |
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.
Reviewed 4 of 31 files at r3, 1 of 2 files at r4.
Reviewable status: 5 of 32 files reviewed, 8 unresolved discussions (waiting on @maxitg and @taliesinb)
Kernel/GeneralizedGridGraph.m, line 32 at r5 (raw file):
(* Implementation *) GeneralizedGridGraph[args___] := ModuleScope[
To change args_
-> dimensionSpecs_, opts : OptionsPattern[]
the CheckArgumentCount
check should probably done in GeneralizedGridGraph
:
GeneralizedGridGraph[args___] :=
Null /; CheckArguments[GeneralizedGridGraph[args], {1, 1}] && False;
then
GeneralizedGridGraph[dimensionSpecs_, opts : OptionsPattern[]] := ModuleScope[...]
We might want to standardize how we do argument check for all our exposed functions.
Kernel/HypergraphToGraph.m, line 43 at r5 (raw file):
(* main *) expr : HypergraphToGraph[ hgraph_,
hypergraph_,
Kernel/IndexHypergraph.m, line 22 at r5 (raw file):
(* main *) expr : IndexHypergraph[hgraph_, start : _ : 1] := ModuleScope[
IndexHypergraph[hypergraph_, startIndex_ : 1]
Kernel/IsomorphicHypergraphQ.m, line 23 at r5 (raw file):
(* main *) expr : IsomorphicHypergraphQ[hgraph1_, hgraph2_] := ModuleScope[
IsomorphicHypergraphQ[hypergraph1_, hypergraph2_]
Kernel/RandomHypergraph.m, line 25 at r5 (raw file):
(* Main entry *) expr : RandomHypergraph[sig_, max_ : Automatic] := ModuleScope[
RandomHypergraph[sizeSpec_, maxVertices_ : Automatic]
Kernel/Subhypergraph.m, line 39 at r5 (raw file):
(* main *) expr : Subhypergraph[arg1_, arg2_] := With[{res = Catch[subhypergraph[HoldForm @ expr, arg1, arg2]]},
arg1_, arg2_
-> hypergraph_, vertexList_
Kernel/ToPatternRules.m, line 70 at r5 (raw file):
toPatternRules[#, caller] & /@ rules; ToPatternRules[rules_] := ModuleScope[
hypergraphRules_
Kernel/WolframModel.m, line 39 at r5 (raw file):
With[{properties = $newParameterlessProperties, stepSpecKeys = Values[$stepSpecKeys]}, FE`Evaluate[FEPrivate`AddSpecialArgCompletion["WolframModel" -> {{"PatternRules"}, 0, stepSpecKeys, properties}]]];
Interesting. So the list of strings autocompletes not only at level 1...
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.
Reviewable status: 5 of 32 files reviewed, 8 unresolved discussions (waiting on @daneelsan and @taliesinb)
Kernel/GeneralizedGridGraph.m, line 32 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
To change
args_
->dimensionSpecs_, opts : OptionsPattern[]
theCheckArgumentCount
check should probably done inGeneralizedGridGraph
:GeneralizedGridGraph[args___] := Null /; CheckArguments[GeneralizedGridGraph[args], {1, 1}] && False;then
GeneralizedGridGraph[dimensionSpecs_, opts : OptionsPattern[]] := ModuleScope[...]We might want to standardize how we do argument check for all our exposed functions.
Why would we want to check in GeneralizedGridGraph
? I think it would be better to do it consistently with throw[Failure[...]]
. I agree wrt consistency, and I think we should use the new messages system for it.
Kernel/HypergraphToGraph.m, line 43 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
hypergraph_,
Done.
Kernel/IndexHypergraph.m, line 22 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
IndexHypergraph[hypergraph_, startIndex_ : 1]
Done.
Kernel/IsomorphicHypergraphQ.m, line 23 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
IsomorphicHypergraphQ[hypergraph1_, hypergraph2_]
Done.
Kernel/RandomHypergraph.m, line 25 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
RandomHypergraph[sizeSpec_, maxVertices_ : Automatic]
Done.
Kernel/Subhypergraph.m, line 39 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
arg1_, arg2_
->hypergraph_, vertexList_
Done.
Kernel/ToPatternRules.m, line 70 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
hypergraphRules_
Done.
Kernel/WolframModel.m, line 39 at r5 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
Interesting. So the list of strings autocompletes not only at level 1...
Yes, it does.
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.
Reviewable status: 5 of 32 files reviewed, 9 unresolved discussions (waiting on @daneelsan, @maxitg, and @taliesinb)
Kernel/GeneralizedGridGraph.m, line 32 at r5 (raw file):
Previously, maxitg (Max Piskunov) wrote…
Why would we want to check in
GeneralizedGridGraph
? I think it would be better to do it consistently withthrow[Failure[...]]
. I agree wrt consistency, and I think we should use the new messages system for it.
Right, also
func[args___] /; !CheckArgumentCount[...] := throw[...]
makes more sense than
Func[args___] := Null /; CheckArgumentCount[...] && False
Kernel/RandomHypergraph.m, line 31 at r6 (raw file):
(* Error messages *) RandomHypergraph::invalidSizeSpec = "\
This message needs to be also updated also in Tests/.
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.
Reviewable status: 5 of 32 files reviewed, 9 unresolved discussions (waiting on @daneelsan, @maxitg, and @taliesinb)
Kernel/RandomHypergraph.m, line 31 at r6 (raw file):
Previously, daneelsan (Daniel Sanchez) wrote…
This message needs to be also updated also in Tests/.
I reverted it back to not break backwards compatibility unnecessarily.
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.
Reviewable status: 5 of 32 files reviewed, 9 unresolved discussions (waiting on @daneelsan, @maxitg, and @taliesinb)
Changes
"HighlightMissingArgumentsWithTemplate"
.The names of the arguments are taken from pattern names in
SyntaxInformation
.This PR adds names to all
SyntaxInformation
except for the built-inRulePlot
thus enabling this feature for all SetReplace symbols.This also adds missing usage messages, syntax information, and syntax autocompletion.
Expands argument names in usage messages to be descriptive so that using autocompleted templates are more informative.
Comments
Null
is due toOptionsPattern[]
. It can be changed to, say,opts
if that pattern is named asopts : OptionsPattern[]
. However, if one does that, some optional arguments will always be displayed in the template, which is, of course, unacceptable. I think we'll have to wait until this weed is fixed upstream:This change is