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

Operater optimization #26

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Operater optimization #26

wants to merge 8 commits into from

Conversation

shigma
Copy link
Collaborator

@shigma shigma commented Nov 10, 2018

Some of the changes are proposals and others are features.

  1. [Adjustment] Add context declarations into expressions for supporting following condition:
entranceFunction = Module[{internalFunc},
    internalFunc[foo_] := bar;
    Return[doSomethingWith[internalFunc]];
]
  1. [Chore] Delete unnecessary regex grouping and quotation marks.
  2. [Feature] Support numbers with base, scientific notation, precision or accuracy. See here.
   11^^11.11`11*^11
(* ^^^^^^^^^^^^^^^^^ constant.numeric *)
  1. [Adjustment] Display built_in_options with variable.parameter. I know this is not that accurate, but much better than variable.function. Any ideas for this scope are welcomed.
  2. [Feature] Support shorthand syntax for Out.
  %%%
(*^^^ storage.type.Out *)
  %12
(*^^^ storage.type.Out *)
  1. [Feature] Support operator for Power.
  2. [Adjustment] Display & with variable.function instead of keyword.operator. I think this would be more appropriate.
  3. [Break] Remove the rule below. I don't know what does a prefix have to do with "constant".
match: ((?:{{symbol}}`)*(\${{symbol}}))(?=\s*:?=)
scope: entity.name.constant.wolfram
  1. [Feature] Support shorthand syntax for Infix.
  a ~ f ~ b ~ g ~ c
(*    ^ variable.function *)
(*            ^ variable.function *)
  1. [Feature] Support operator for Definition and FullDefinition.
  2. [Adjustment] Better display with specific functions, for example:
  Sum[x, {x, 1, 100}]
(*        ^ variable.parameter *)
  Map[func, list]
(*    ^^^^ variable.function *)
  TakeWhile[list, func]
(*                ^^^^ variable.function *)

These functions are currently listed as variables. With further development of branch new-build-system, we can easily generate them within lines of code.

@@ -278,8 +357,22 @@ contexts:
- include: sequence
- match: (?=[^\[])
pop: true
- match: ((?:{{symbol}}`)*(\${{symbol}}))(?=\s*:?=)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule was here to add to the index global variable declarations

$GlobalValue = xxx;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this rule was not grammatical. System global varibles are defined by "built-in" variables. General varibles begin with "$" are not necessarily a "constant".

- include: expressions

expressions:
- include: declarations
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am no sure we should index all the localized functions.

Copy link
Collaborator Author

@shigma shigma Nov 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "index" mean?

From my perspective, "declarations" have no grammatical difference with other expressions and should not be ruled out.

@@ -90,14 +106,19 @@ contexts:
pop: true

literals:
- match: (?:\d+\.?|\.\d)\d*\`{0,2}
# numbers, with base, scientific notation, precision or accuracy
- match: ([0-9]+\^\^)?{{number}}(\`\`?({{number}})?)?(\*\^{{number}})?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first {{number}} can contain letters to represent digits in bases higher than 10.
I was working on this syntax but we can do it here as well. This would be my proposal

Suggested change
- match: ([0-9]+\^\^)?{{number}}(\`\`?({{number}})?)?(\*\^{{number}})?
- match: |-
(?x)
(?:(\d+)(\^\^))
((?:[$[:alnum:]]+\.?|\.[$[:alnum:]])[$[:alnum:]]*)
(?:(\`\`?)({{number}})?)?
(?:(\*\^)([+-]?\d+))?
captures:
1: constant.numeric.base.wolfram
2: keyword.operator.base.wolfram
3: constant.numeric.wolfram
4: keyword.operator.precision.wolfram
5: constant.numeric.precision.wolfram
6: keyword.operator.exponent.wolfram
7: constant.numeric.exponent.wolfram
- match: |-
(?x)
({{number}})
(?:(\`\`?)({{number}})?)?
(?:(\*\^)([+-]?\d+))?
captures:
1: constant.numeric.wolfram
2: keyword.operator.precision.wolfram
3: constant.numeric.precision.wolfram
4: keyword.operator.exponent.wolfram
5: constant.numeric.exponent.wolfram

I know it does not distinguish between precision and accuracy but I did not want to explode the pattern even more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And exponents can have a decimal point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

((?:[$[:alnum:]]+.?|.[$[:alnum:]])[$[:alnum:]]*)

What is $ for?

Copy link
Collaborator Author

@shigma shigma Nov 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I don't like the idea to treat ^^ and *^ as "operators". If they were operators, they should have precedence and expressions like 8 ^^ 10 should be right. But obviously not.

- match: ((?:System`)?{{built_in_numeric_constants}})
scope: constant.numeric.wolfram
- match: ((?:System`)?{{built_in_constants}})
scope: constant.language.wolfram
- match: ((?:System`)?{{built_in_options}})
scope: variable.function.wolfram
scope: variable.parameter.wolfram
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is conceptually wrong but I think function was there to have the same colouring for all System symbols.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But built-in-constants are displayed as "constants".

WolframLanguage.sublime-syntax Outdated Show resolved Hide resolved
WolframLanguage.sublime-syntax Outdated Show resolved Hide resolved
@shigma
Copy link
Collaborator Author

shigma commented Nov 15, 2018

Updates

  1. Optimize number pattern, support scope names with base, precision, accuracy and exponent.
  11`
(*^^ constant.numeric *)
(*  ^ constant.numeric.precision *)
  11.`11
(*^^^ constant.numeric *)
(*   ^^^ constant.numeric.precision *)
  .11``
(*^^^ constant.numeric *)
(*   ^^ constant.numeric.accuracy *)
  11.11``
(*^^^^^ constant.numeric *)
(*     ^^ constant.numeric.accuracy *)
  11^^1a
(*^^^^ constant.numeric.base *)
(*    ^^ constant.numeric *)
  11.11*^-11
(*^^^^^ constant.numeric.wolfram *)
(*     ^^^^^ constant.numeric.exponent *)
  1. Optimize parameter pattern, support scope names with blank, head and default (which has not been supported in the past).
  _.
(*^^ variable.parameter.default *)

  var_head
(*^^^^^^^^ meta.parameter *)
(*^^^ variable.parameter *)
(*   ^ variable.parameter.blank *)
(*    ^^^^ variable.parameter.head *)

  var__head : foo
(*          ^ meta.pattern keyword.operator.Optional *)

  var___head ? EvenQ
(*           ^ meta.pattern keyword.operator.PatternTest *)

  var: patt ? EvenQ : foo
(*^^^ variable.parameter *)
(*   ^ keyword.operator.Pattern *)
(*     ^^^^^^^^^^^^ meta.pattern *)
(*     ^^^^ variable.other*)
(*          ^ keyword.operator.PatternTest *)
(*            ^^^^^ variable.function *)
(*                  ^ keyword.operator.Optional *)
(*                    ^^^ variable.other *)
  1. Remove function classifications, which will be discussed in another PR.
  2. Add DynamicModule into scoping_functions.
  3. Merge pattern-short into variables for better semantics.
  4. Fix a pattern capture mistake.
  5. Add some comments.

@chere005
Copy link
Collaborator

Re 7 I would keep & an operator.. There's some good features in this PR, but a lot to go through all at once. Smaller PRs might make it easier to get things merged since we can't approve it until all changes are OK. I see good work on supporting : as Optional, I haven't had a chance to check this out and try it yet but does it work for recognizing foo : (_Integer | _String) as pattern? I'll try to look at this more carefully in the next couple weeks.

@chere005
Copy link
Collaborator

I did install it to start looking at things actually... first issue:

If[rowStart > 1,
	rowList = Replace[rowList, x_Integer :> x - rowStart + 1, {1}];
];

@chere005
Copy link
Collaborator

Another issue:

Module[{data, fileMode, dims, enc, dataStrm, ignoreEmptyLines, summary},

	(* Load LibraryLink functions *)
	If[!loadAdapter[],
		Return@CreateSVToolsException["AdapterNotLoaded"];
	];

	enc					= ValidateOption[True, SVGetDimensions, CharacterEncoding, Alternatives @@ Join[System`$CharacterEncodings, {None, Automatic, "UTF8ISOLatin1"}], opts];
	ignoreEmptyLines	= ValidateOption[True, SVGetDimensions, "IgnoreEmptyLines", True | False, opts];
	td					= ValidateOption[True, SVGetDimensions, "TextDelimiters", t_String /; 0 <= StringLength[t] <= 1, opts];
	summary				= ValidateOption[True, SVGetDimensions, "Summary", _?BooleanQ, opts];
]

@chere005
Copy link
Collaborator

Re 7 I would keep & an operator.. There's some good features in this PR, but a lot to go through all at once. Smaller PRs might make it easier to get things merged since we can't approve it until all changes are OK. I see good work on supporting : as Optional, I haven't had a chance to check this out and try it yet but does it work for recognizing foo : (_Integer | _String) as pattern? I'll try to look at this more carefully in the next couple weeks.

I tested the : as pattern and it works

@chere005
Copy link
Collaborator

This is causing MAJOR performance detriments on my machine. I couldn't even open a file with 10k+ lines of code, but when I switched to the plugin on master it loaded instantly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants