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

Feature suggestion: Calc variables #354

Closed
DrorHarari opened this issue Jan 6, 2019 · 15 comments

Comments

@DrorHarari
Copy link

commented Jan 6, 2019

Calc Variables Feature Description

When working with Keypirinha's Calc plugin, one may want to save the result of the calculation for later use in other calculations. This features enables that using one the following patterns:

var=expr
or:
expr=var

For example:

image

image

image

image

The variables are stored persistently in a file variables.jsonlocated in the plugin's local folder.

Enjoy

Original proposal:

Proposal to allow Calc to save results as variables for later use.

Examples:
12+3 > a => 15
a/5>b => 3
a>b => 1 (the space after b disables the 'save to b' so this checks if a>b)
'abcd' >c => abcd
len(c) => 4
c+'x' => abcdx
2+1>pi => Error: cannot override constant value

Notes:

  • Variables names must start with an alphabetic character and may also contain digits.
  • Variable name on saving cannot have trailing spaces. If spaces trail variable name, it is ignored.
  • Variables are transient and are gone once the Calc plugin reloads.

I have implemented this (+19 lines) and its works nicely. If it's of interest, I'll submit a PR.

@polyvertex

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

Neat idea! You need not to "ask" before submitting a PR you already implemented :)

I would refrain overriding any of the implemented operators though as it may feel counter-intuitive to some.
Perhaps @ or even # if possible but I cannot recall how painful it would be to support an extra operator based on a new token, if possible at all. Which would mean the expression would need to be pre-parsed first. Very much doable it seems :)

A word of caution though: a plugin may be re-loaded at runtime by KP for whatever reason (e.g. package update) in which case user would silently loose their defined variables. So you may want to keep them persistent in package's dedicated cache directory. Cache would be reset-able via an additional command or item. Or perhaps automatically reset each time KP is invoked/shown.

@DrorHarari

This comment has been minimized.

Copy link
Author

commented Jan 7, 2019

I would refrain overriding any of the implemented operators though as it may feel counter-intuitive to some.

Many KP users likely have command line experience (surely not all) and for them the redirect to symbol > would ring familar (redirect the result to a variable).

Perhaps @ or even # if possible but I cannot recall how painful it would be to support an extra operator based on a new token, if possible at all

I wonder how much KP users really use the > operator in their calculations (e.g. how often do they try to calculate "4>pi") and whether it would not be better keeping the pattern I devised, especially since if they need to evaluate it they could append a space as in "4>pi "

I think that the main problem with these is that the simpleeval package used in Calc relies on the Python AST so it's not clear that we can have support for # or @ as that's not standard Python.

You can view the 'dumb' implementation at:
DrorHarari/Packages@060b998

It does not rely on the AST parsing at all.

A word of caution though: a plugin may be re-loaded at runtime by KP for whatever reason

Good idea. Will implement.

Cache would be reset-able via an additional command

I wanted to minimize the change. Can add this too adding Clear Mem command.

@polyvertex

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

It's more about making the intent clear. The Calc package is advertised as a mathematical expression evaluator, so it should remains even though the usage of the > operator clearly won't equal + in terms of frequency. But at least it allows me to copy-paste some expression and have the result I expect.

Also, what I meant to say above is that since the format of the expression - as you defined it - to assign a value to its variable is well-known, it would be easy to pre-parse the expression before sending it to simpleeval so that the >c part of the expression could be stripped first. That way, any reasonable kind of sequence could be used. E.g. : c. A very strict regex would do in that case :)

@DrorHarari

This comment has been minimized.

Copy link
Author

commented Jan 7, 2019

You know best how it's advertised 😎 - from my perspective it looks more like a python expression evaluator with its support for strings etc. If you prefer the syntax 12+3:a over 12+3>a then I will change it.

The code currently uses a very strict regex to split the variable name from the expression:
r'(?P<expression>[^>]+)>\s*(?P<save_var>[a-zA-Z][a-zA-Z0-9]*)$'
It is trivial to swap > for : at the end - I think that the colon in Python is only used to delimit statements so I won't expect a python expression to end with :name.

@DrorHarari

This comment has been minimized.

Copy link
Author

commented Jan 9, 2019

I have changed the save character from > to : and it works though I find it less intuitive and I still think > was better.. The only problem I see is incidental:

image

It triggers a FileNotFoundError error message in the FileBrowser pacakge (in ismount). Not sure if this matters.

On the other hand, as variables are now stored, there is also:
image

@dreadnaut

This comment has been minimized.

Copy link

commented Feb 9, 2019

Are two-character symbols possible? What about ->, as in 12 + 3 -> a ?

It would be memorable (an arrow!), visually recognisable, and clear any ambiguity with > as "greater-than".

@DrorHarari

This comment has been minimized.

Copy link
Author

commented Feb 9, 2019

@dreadnaut for now I implemented ':' as the default save-to-variable delimiter (per @polyvertex preference). This can be modified in the config but only to a few other single characters. I feel that it needs to be fast to type and -> is too much finger muscle...

BTW, you can play with the version with the variables - it's at https://github.com/DrorHarari/keypirinha-playground/releases/tag/0.1

@sam97

This comment has been minimized.

Copy link

commented Feb 14, 2019

+1 for pointing out the mathematical nature of the Calc plugin. We might as well run a Python shell to get full flexibility at this point. 😃

Not against the idea though, especially since the "dumb" implementation seems to be surprisingly minor in changes. But care should be taken that potential future symbols do not get stumped. I see that the colon is used to portray ratios in mathematics. If ratios are ever to be brought into the plugin, this will create problems then.

I'd suggest using the := symbol, since it is more in line with mathematical expressions and having : and = next to each other does not make much sense in mathematics (so no worries of stumping anything).

I feel that it needs to be fast to type and -> is to much finger muscle.

There's a C developer joke to be made here, but I'll leave it for the witty.

@DrorHarari

This comment has been minimized.

Copy link
Author

commented Feb 14, 2019

@sam97 I wouldn't worry too much about ratios or other potential revolutions to the colon symbol in Python as it is already there in 3.8 with assignment expressions (https://www.python.org/dev/peps/pep-0572/#examples-from-the-python-standard-library):

Current:

env_base = os.environ.get("PYTHONUSERBASE", None)
if env_base:
    return env_base

Improved:

if env_base := os.environ.get("PYTHONUSERBASE", None):
    return env_base

So definitely writing 2048*45 := a will be very counter intuitive in Python and thus using > which is something that is familiar from the command line (likely dear to most KP users) makes more sense. I still have ':' as the default and '>' as an option. I'll be happy to take the option out and settle on whatever @polyvertex prefers.

@dreadnaut

This comment has been minimized.

Copy link

commented Feb 14, 2019

I still have ':' as the default and '>' as an option.

You can't use > as assignment, without making the syntax ambiguous:

21 > 10         // keypirinha responds "1", because the expression is true
3 * 2 * 7 > a   // is this assigning a = 42, or comparing 42 and the value of "a"?
12 > a          // and if the variable "a" exists, would this compare or reassign?
@DrorHarari

This comment has been minimized.

Copy link
Author

commented Feb 14, 2019

@dreadnaut while I haven't had use for greater-than calculations, I assume that there are some people who find this a compelling use case for Calc but are still not inclined to use <= instead of > with the logic reversed. For those people I was playing with insisting on anything to the left of the > symbol be a valid variable name (not even a padding whitespace) so that you could always use 3 * 2 * 7 > a␣ to check if 42 is greater than a. The current version does not have this implemented but it is a trivial regex change to bring it back.

@polyvertex

This comment has been minimized.

Copy link
Member

commented Feb 24, 2019

@DrorHarari I hear your concern about the number of keystrokes but both < and > being valid and implemented operators already, we have to find a replacement for this assignment feature to what was your default >. I think @dreadnaut and @sam97 offered good suggestions.

So to synthesize:

  • Assignment could be specified as a suffix or as a prefix. That is <varname><op><expression> or <expression><op><varname>
  • In which case prefix and suffix operators should be different logical entities - i.e. separate settings; configured operators must not be equal. Like in varname <- 3 or 33 -> varname
  • This "assignment" feature should be implemented as the parsing of the global expression - typically a strict regex - instead of being implemented as new operator(s) supported by the underlying simpleeval library. So that things don't get messy in the code and this feature stays in its corner and could even be easily disabled and disable-able.
  • This regex should not be strict on having the assignment operator space-separated
  • If a prefix assignment operator is specified, the remaining part of the expression should be ignored by your code. That is, if a user specified both a prefix and a suffix assignment operator, only the prefix assignation should be extracted from the typed expression, leaving the full remainer - including the suffix assignment operator part to simpleeval, so that user notices it is incorrect.

Side notes:

  • I think settings related to the assignment operators should be interpreted as enums with very limited accepted options. Typically: ('<-', ':=') for the prefix op and ('->', '::', '#') for the suffix one.
  • I think # makes sense as a single-character suffix assignment operator, as in "hahtag myvar" :)
  • Special bonus to @DrorHarari regarding the suffix operator: since you seem to be very comfortable with having > as a suffix assignment operator and since it is your PR, perhaps you could define the enum of the suffix operator as ('->', '::', '#', '>') instead. But please keep the '>' operator undocumented and disabled by default! :) (I would ask you though to clearly mark it in the code as a special treat to the author so that reviewers do not get tempted to document it)

Thanks for your patience

@DrorHarari

This comment has been minimized.

Copy link
Author

commented Feb 24, 2019

@polyvertex - I am easy to convince with good arguments. Here's my take:

  • Given Python 3.8 expression assignment syntax v := expr I think it makes much sense to use it (that's the <varname><op><expression> part) without options. I think it should be easy to integrate it with the current = recognizer.
  • For the <expression><op><varname> (to extent we deem it desirable) my preference would be to the # character for the save-to-variable operator. No chance for future conflict with Python as it is the comment syntax. I'd be totally fine with no alternative syntax here too.
  • My parsing was done with a strict regex and I see no problem implementing the above the same way.
  • As for the 'special bonus', I don't need any. I prefer coherence, my fingers will learn.
  • The different prefix vs. suffix operator is a bit annoying - could we consistently use # for both?

If we agree to the above, I'll find some time to redo the PR...

@DrorHarari

This comment has been minimized.

Copy link
Author

commented Mar 3, 2019

Over the weekend I have changed the variable feature to use the following patterns:
var=expr
or:
expr=var

It feels very natural on both sides and I don't see a need for variation.

I will send a new PR. What makes the change long include:

  • Support for the always_evaluate setting (naturally integrated with the variable support)
  • Preventing calculation errors from showing too early (will show after there is a =)
  • Disambiguating = and ==
  • Variable actions (show, copy, delete, delete_all)
  • Preventing constants and Python keywords from being used as variables
  • Saving and restoring from a variables.json file ('ans' is currently also stored? is it a problem?)

BTW @sam97, this solution seems to be light on my lazy finger...

image

image

image

image

@DrorHarari DrorHarari referenced this issue Mar 3, 2019

@polyvertex polyvertex added status/done and removed status/done labels Aug 15, 2019

@polyvertex

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

@DrorHarari's patch is now shipped with v2.24. Thanks!

@polyvertex polyvertex closed this Aug 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.