dictation-toolbox / Caster

Dragonfly-Based Voice Programming and Accessibility Toolkit
https://dictation-toolbox.github.io/Caster/
Other
340 stars 121 forks source link

Removing `R()` from command spec #672

Open alexboche opened 5 years ago

alexboche commented 5 years ago

Is your feature request related to a problem? Please describe. Caster commands re surrounded by the R(). I think this is a barrier that discourages non- caster dragonfly users from using and contributing to Caster. Certainly not a big barrier but one nonetheless. I have heard somebody say that their commands would not work with caster because caster "has a different syntax" than dragonfly; presumably they were referring to the R(). Adding the R is also just an extra step in writing commands that slows people down a little.

I don't have a good understanding of the purpose of R, but it seems possible to me that someone could write something that loops over all the commands and automatically carries out the functionality that R now does one at a time. Though that would not work if each command requires unique metadata which it sounds like is a big part of what R does.

What is R for? Here is synkarius' recent description of the purpose of R from the issue page on reload ability of grammars:

As for R, I agree that it would be nicer to be able to do without it. I am not going to get rid of it as a part of this rewrite, but if anyone is interested, this is what needs to be replaced:

It adds metadata to actions. I had grander dreams of what I wanted to do with this metadata once in terms of GUI windows/components, but clearly that never really happened. So the usefulness of that metadata is somewhat questionable now. It enables ContextSeeker and AsynchronousAction. ContextSeeker was ultimately too complicated and never got used much except by AsynchronousAction, which simplified it somewhat and I think is generally useful.

Describe the solution you'd like

  1. Write up a clear description of the value provided by R
  2. Consider removing the "R" from command definitions ( at least new commands being created).
  3. Possibly find a way to automatically apply the R to all commands.
  4. Possibly keep R for only a small subset of commands for which it is most important. It sounds like this may be the commands that use AsynchronousAction.

@lexxish also expressed interest in taking out the R. I should say that I don't really feel qualified to say much more about this issue. I am more just writing this to bring the topic to the attention of others.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/78569416-removing-r-from-command-spec?utm_campaign=plugin&utm_content=tracker%2F1825907&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F1825907&utm_medium=issues&utm_source=github).
synkarius commented 5 years ago

Let me be a little more explicit about what I meant with "enables ContextSeeker and AsynchronousAction". If you look in stack.py, you'll see the rube goldberg machine that I built years ago with the intentions of letting information from past commands persist awhile in order to influence the execution of future commands. This is a feature which never really took off, but upon which, unfortunately, I built AsynchronousAction, which does get used.

So basically, if someone can build an "AsynchronousAction2" or something like that, which does not depend on ContextSeeker, you can get rid of R, and basically all of stack.py. All it really needs to do is perform some action on a timer and be cancelable either by voice or by some user-specified condition.

In doing so, you would also lose the "rdescript" command descriptions, but A) these have been automated rather than handcrafted in many cases, and B) the project to incorporate "rdescript" in some GUI project could be done in some other way, so the value lost would be, IMO, negligible.

I really want stack.py to die. I think it is the ugliest part of Caster, and would be glad to either guide this feature or do it myself after I finish the merger rewrite and take a short break to get some other stuff done.

LexiconCode commented 5 years ago

@mrob95 considered automatically converting items to RegisteredActions. At the time I wasn't fully convinced to automate conversions however I think I've been convinced otherwise since then.

However reimplementing AsynchronousAction then dropping Context Stack sounds like it would clean up the CodeBase. Losing rdescript would be difficult without its functionality replaced. Simply because some commands aren't self-explanatory. All in all stack.py should go to the chopping block.

synkarius commented 5 years ago

@LexiconCode You make a good point about some commands' actions not being self explanatory. And it's certainly easier to have that in the code rather than just in the docs, so users don't have to flip back and forth to match things up.

mrob95 commented 5 years ago

I'm all in favour of this. The guiding principle of the last few changes I've made (star imports, rule loading functions, removing rdescripts) has been to cut down the volume of code required to define grammars, making it easier for users to read through them and iterate. I think the R() syntax is the last thing which feels unnecessary and clunky.

There may be some cases in which rdescripts are useful, but I think I would be willing to lose the command-labelling ability if it meant a reduction in grammar complexity across the code base. If a command needs labelling them this can be done with comments.

I'm not too familiar with the context stack, my naïve approach to this was just to do something like:

for k, v in self.mapping.items():
    if not isinstance(v, RegisteredAction):
        self.mapping[k] = RegisteredAction(v)

in merge rule, and then replace any remaining mapping rules with merge rules so that this is applied to every command on start-up. Doing this produces a circular import issue, but IIRC it would have been reasonably simple to sort out.

LexiconCode commented 3 years ago

Let me be a little more explicit about what I meant with "enables ContextSeeker and AsynchronousAction". If you look in stack.py, you'll see the rube goldberg machine that I built years ago with the intentions of letting information from past commands persist awhile in order to influence the execution of future commands. This is a feature which never really took off, but upon which, unfortunately, I built AsynchronousAction, which does get used.

So basically, if someone can build an "AsynchronousAction2" or something like that, which does not depend on ContextSeeker, you can get rid of R, and basically all of stack.py. All it really needs to do is perform some action on a timer and be cancelable either by voice or by some user-specified condition.

In doing so, you would also lose the "rdescript" command descriptions, but A) these have been automated rather than handcrafted in many cases, and B) the project to incorporate "rdescript" in some GUI project could be done in some other way, so the value lost would be, IMO, negligible.

I really want stack.py to die. I think it is the ugliest part of Caster, and would be glad to either guide this feature or do it myself after I finish the merger rewrite and take a short break to get some other stuff done.

This is another place where a grammar/spec API can be of use. The API not only could reference the underlined spec in action but be used in the context stack or something similar. I think the implementation would be much simpler.