dictation-toolbox / Caster

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

Proposal for reducing boilerplate, folder structure and loading rules. #589

Closed mrob95 closed 4 years ago

mrob95 commented 5 years ago

I haven't been particularly active recently, but have been bouncing ideas around my head for a while regarding the direction and structure of caster. Going to do a brain dump here in the hope that it prompts some discussion and hopefully consensus.

Where we are

I'm going to refer in this section to old caster and new caster.

Old caster is anything before the move to pip, and is basically a known entity. You install it by manually downloading the git repo and then manually setting the natlink user directory, and all grammars are stored and loaded from that directory. It has the advantage of being reasonably conceptually simple (everything in one directory), but is a little bit cumbersome to set up and does not make a clear distinction between user space and backend.

New caster is anything after the move to pip. The first thing to say about this is that however new caster was supposed to look, it is unfinished and has a few issues. Firstly, if installed using pip we have gone from caster being in one place (wherever the user downloaded it) to three places - _caster.py in the Natlink directory, most of the code in the Python packages directory, and the .caster user space containing settings and filters. I think this is really confusing. New users need to be able to easily find where the code is and be able to see how they can change it to do what they require. Secondly, because the instructions in the develop branch describe installation using pip, but we are not yet able to recommend this, develop and master are basically unmergeable and are beginning to diverge significantly in functionality.

At the moment I would say that we are stuck somewhere between old caster and new caster. So the question is: what do we have to do to new caster to get it to the point where it can replace old caster?

Proposals

Structure, Rule loading

I think this can mostly be done by amending the UserContentManager class.

My hope is that user's individual .caster folders can then be uploaded as their own mini projects from which others can gain inspiration without having to pull request against the main repo, but whether this is realistic remains to be seen.

Reducing boilerplate Done

I think most people have probably noticed that to create a grammar in caster requires a fair bit of boilerplate/repeated/unnecessary code. To take for example the Firefox grammar, to implement 16 commands requires ~70 lines of code, much of which is not actually describing commands. Three proposals for fairly easily cutting this down:

if settings.SETTINGS["apps"]["firefox"]: if settings.SETTINGS["miscellaneous"]["rdp_mode"]: control.nexus().merger.add_global_rule(FirefoxRule()) else: rule = FirefoxRule(name="firefox") gfilter.run_on(rule) grammar.add_rule(rule) grammar.load()

this can easily be abstracted so that rules can be added with just something like `control.app_rule(FirefoxRule(), context)` 

- [x] #590 Centralise imports. At the moment every file has basically the same ~8 or so lines at the top which are just importing the necessary bits and pieces to make a rule. These can all just be put in one file so that at the top of every rule all we need is:

from castervoice.lib.imports import *



---

Thoughts? Happy for people to use this issue as a place to put any general thoughts they have about the direction/structure of the project, there is probably a lot that I'm missing. It might also be good to get people's perspectives on what the priorities should be for the project and where the pain points are.

Ideally if we are going to make big changes it would be better to do them all at once rather than repeatedly breaking things.
kendonB commented 5 years ago

Tidy up the folder structure. I think this is possibly the most important thing to do as it is a mess at the moment and it's hard to think clearly about changing things when some of the rules are in cv/apps all together, others are in cv/lib/ccr in their own subdirectories, etc. Moving everything into one cv/rules folder with a consistent structure will make it much easier to load core rules alongside user rules. Unfortunately this will probably break most of the active pull requests.

Agree. Let's do it and break the PRs. It won't be hard to adjust them. Before we do that though, let's 1) be clear about what the full file structure, including working out how the new app standards will fit in: https://github.com/dictation-toolbox/Caster/pull/530.

Simplify the process of creating user rules in user space so that they have exactly the same syntax as any other rule, removing the get_rule formulation. Allow rules in user space with the same name to override caster defaults. So for example if you copy python.py into user space the Python rule will be loaded from that file instead of the default. You can then make changes to that file while still allowing the repository to be updated through pip or git. Simplify this process further by adding voice commands to copy the file and open in a text editor, e.g. edit Python grammar.

Beautiful ideas. Agree. Probably also good to have an "update Python grammar" function that merges in changes from the default to your file.

kendonB commented 5 years ago

Let's also think about having a "commands" folder that completely separates out the spoken parts from the functionality.

kendonB commented 5 years ago

What about this? There are probably mistakes for the caster infrastructure stuff. All beneath castervoice:

- rules
    - core
        - nav.py
        - app_control.py # use this to split out stuff from nav.py and app grammars that are common. 
                         # e.g. "save", "open"
        - alphabet.py
        - numbers.py
        - punctuation.py
        - text_manipulation.py
    - browser
        - browser.py # contains shared commands (e.g. Ctrl-L) is the same in chrome and firefox
        - chrome.py
        - firefox.py
    - editor
        - editor.py # contains most common shared commands (e.g. Ctrl-G + number + Enter is 
                    # usually go to line)
        - atom.py
        - eclipse.py
        - emacs.py
        - flashdevelop.py
        - jetbrains.py
        - kdiff3.py
        - lyx.py
        - msvc.py
        - notepadplusplus.py
        - rstudio.py
        - sqldeveloper.py
        - ssms.py
        - sublime.py
        - typora.py
        - visualstudio.py
        - vscode.py
    - windows
        - explorer.py
        - file_dialogue.py
    - terminal
        - terminal.py
        - cmd.py
        - gitbash.py
    - mouse
        - griddouglas.py
        - gridlegion.py
        - gridrainbow.py
    - speech_engine
        - dragon.py
        - wsr.py
    - other
        - adobe_acrobat.py
        - excel.py
        - fman.py
        - foxitreader.py
        - githubdesktop.py
        - gitter.py
        - totalcmd.py
        - winword.py
        - sikuli.py # needs to be pulled out from the current file
- commands
    - literally identical to rules and contains the command phrasings only as dictionaries
- doc
    - as is
- functions # for functions that support actual actions
    - core
        - actions.py
        - alphanumeric.py
        - clipboard.py
        - context.py
        - control.py
        - gdi.py
        - github_automation.py
        - github_automation.ahk
        - navigation.py
        - settings.py
        - temporary.py
        - terminal.py
        - text_manipulation.py
        - text_utils.py
        - textformat.py
        - utilities.py
        - version.py
    - homunculus # from asynch
    - mouse # from asynch
        - existing files + lib/dll/tirg-dll.dll # possibly get the source code in here too
    - sikuli # from asynch
    - other
        - settingswindow.py  # from asynch
- infrastructure # for actual caster infrastructure
    - ctrl # from lib
    - dfplus
- bin
    - as is
- tests
    - dev # from lib/dev
    - plus what else is already there
mrob95 commented 5 years ago

There is definitely no rush and I agree that it would be good to decide on a folder structure which makes sense for everybody and will scale well. One thing worth bearing in mind is that at the moment we have two different ways of loading rules from within folders, in the init files of cv/apps and cv/lib/ccr respectively:

def is_valid(module):
    ''' This function attempts to import the applications in order to detect
    errors in their implementation . After they are imported, they are garbage collected
    when the function returns.'''
    try:
        _ = __import__(module, globals(), locals())
        return True
    except Exception as e:
        print("Ignoring application '{}'. Failed to load with: ".format(module))
        utilities.simple_log()
        return False

modules = glob.glob(os.path.dirname(__file__) + "/*.py")
# only valid applications will be added to the list
__all__ = [
    os.path.basename(f)[:-3]
    for f in modules
    if (not f.endswith('__init__.py') and is_valid(os.path.basename(f)[:-3]))
]

And

command_sets = {
    "bash.bash": ("Bash", ),
    "core.alphabet": ("Alphabet", ),
    "core.nav": ("Navigation", ),
    "core.numbers": ("Numbers", ),
    "core.punctuation": ("Punctuation", ),
    "core.text_manipulation": ("TextManipulation", ),
    "cpp.cpp": ("CPP", ),
    "csharp.csharp": ("CSharp", ),
    "dart.dart": ("Dart", ),
    "voice_dev_commands.voice_dev_commands": ("VoiceDevCommands"),
    "go.go": ("Go", ),
    "haxe.haxe": ("Haxe", ),
    "html.html": ("HTML", ),
    "java.java": ("Java", ),
    "javascript.javascript": ("Javascript", ),
    "latex.latex": ("LaTeX", ),
    "markdown.markdown": ("Markdown", ),
    "matlab.matlab": ("Matlab", ),
    "python.python": ("Python", ),
    "r.r": ("Rlang", ),
    "rust.rust": ("Rust", ),
    "sql.sql": ("SQL", ),
    "prolog.prolog": ("Prolog", ),
    "vhdl.vhdl": ("VHDL", ),
}

for module_name, class_name_tup in command_sets.iteritems():
    for class_name in class_name_tup:
        try:
            module = __import__(module_name, globals(), locals(),
                                [class_name])  # attempts to import the class
            globals()[class_name] = module  # make the name available globally

        except Exception as e:
            print("Ignoring ccr rule '{}'. Failed to load with: ".format(class_name))
            utilities.simple_log()

These are both kind of hacky and ad hoc. It would be good to get to a point where we can just call one method to load all the files from a particular folder, because this will make it much easier to load from multiple different places and keep track of where things are. The UserContentManager class provides a pretty good base from which this could be done.

kendonB commented 5 years ago

See @lexxish's change here too: https://github.com/dictation-toolbox/Caster/blob/48e8f7c2a3c9f97a3d1b8e6af8ceae4f9b754094/castervoice/apps/__init__.py

kendonB commented 5 years ago

I really don't want to lose the energy here so we should make some concrete progress. @mrob95:

mrob95 commented 5 years ago

What would be the difference between the rules and the commands folder?

Handling conflicts for individual commands is a whole different ballgame which I'm not going to try to do here, this is just about loading rules. The algorithm within UserContentManager will be something like this, assuming we have a rules folder in user space (.caster) and a rules folder in castervoice:

The actual code for importing the rules will just be a modified version of what is already there, basically just globbing for filenames.

kendonB commented 5 years ago

rules will not contain any phrasings, only behavior. commands will only contain phrasings and the minimal code required to move phrasings around. Similar to the SymbolSpecs stuff and how @lexxish has been doing it in his PR.

mrob95 commented 5 years ago

I think messing with the folder structure should wait until #530 is merged. It should not be too difficult to pull the same trick with the separated phrasings and behaviour: if the file has been copied to user space then it will override the default.

Second section added to the OP addressing boilerplate.

comodoro commented 5 years ago

I support 100% the consolidation of folders (@kendonB 's structure seems reasonable, while I do not understand the distinction between functions and infrastructure (libIs shorter, too) and it does not seem nice to mix tirg-dll.dll with the source). Having a separate structure for specifications ("specs") may be advantages for translations and documentation, but it complicates things and there are filter rules to consider.

As for the removal of rdescripts, I have already expressed my opinion, I think these will be missing for automatic grammar documentation at least. Removal of boilerplate code is nice for the users, but it may not be advantageous for the entire code base.

LexiconCode commented 5 years ago

Could we add checkboxes -[ ] for tracking individual components of issue tracking implementation?

mrob95 commented 5 years ago

@comodoro I appreciate the concern and I hope that over time you'll be convinced :-).

Regarding starred imports, I understand the reasons why they are usually frowned upon and agree that they are normally not appropriate. In our case though, where there are a well-defined group of objects which come up again and again and which users can be expected to become familiar with, I think it makes sense. I am already finding it easier to work on the code base and test things out without worrying about whether or not I've remembered to import a particular dragonfly object or w/e.

Regarding rdescripts, I think that the improvement in readability and the reduction in grammar creation overhead is worth the loss of any utility that they provided. I agree that automatic generation of docs would be ideal, and I'm reasonably confident that it can be done adequately without requiring contributors to handwrite rdescripts for every command (which kind of reduces the time saved by generating docs).

Restructuring is definitely on the to-do list.

LexiconCode commented 5 years ago

Regarding rdescripts, I think that the improvement in readability and the reduction in grammar creation overhead is worth the loss of any utility that they provided. ...snip

There is definitely work to be made with generating command history. I don't think we are necessarily giving up on the ability to automatically generate docs either through files or through a GUI.

Overall our current implementation of command history is a step forward. However I do think we need to clean up and add some rdescripts.

Simply saying the word 'up' produces Navigation: (<mtn_dir> | <mtn_mode> [<mtn_dir>]) [(<nnavi500> | <extreme>)], up, None, up, 1, None.

Which is essentially Navigation: (<up> | <None> [<up>]) [(<1> | <None>)],

Then simplifies down to Navigation: up 1

@comodoro I assume part of your thoughts is in regards to the example above. Feel free to clarify. I'm running under the assumption that the downsides of the current implementation can be mitigated by adding rdescripts as needed.

For those commands that have a bunch of extra/choices we might want to override with rdescripts. Mainly at some of the core such as navigation that would benefit the most from tweaking.

Thoughts @mrob95 @comodoro?

comodoro commented 5 years ago

Simply saying the word 'up' produces Navigation: ( | []) [( | )], up, None, up, 1, None.

Which is essentially Navigation: ( | []) [(<1> | )],

Then simplifies down to Navigation: up 1

@comodoro I assume part of your thoughts is in regards to the example above. Feel free to clarify. I'm running under the assumption that the downsides of the current implementation can be mitigated by adding rdescripts as needed.

I am probably behind, could you please explain what you mean? I don't have any command up,Is there a context to it?

My concern is simply that if you want to automatically document grammars, you have to have something that describes them. Almost all current specs, sauce or dunce, for example, are in themselves just meaningless words.

LexiconCode commented 5 years ago

I am probably behind, could you please explain what you mean? I don't have any command up,Is there a context to it?

No you're not behind I forgot I have some filter rules running sauce -> up when dictating.

My concern is simply that if you want to automatically document grammars, you have to have something that describes them. Almost all current specs, sauce or dunce, for example, are in themselves just meaningless words.

@comodoro That's a valid point. The majority of obscure words are contained in core and less so in the app grammars. As a temporary measure I propose that we add back some rdescripts. The guiding rule being simplifying complex commands as demonstrated above or explaining ambiguity. Overall were still left with the benefit of not having that document self-explanatory commands in the code as rdescripts. However there's any other thoughts on how to mitigate these issues feel free to broaden my horizons.

However this brings up the future of casters grammars which have just written a new issue which is relevant to this discussion https://github.com/dictation-toolbox/Caster/issues/625 which would remove the problem of ambiguous commands.

comodoro commented 5 years ago

That's the thing, I do not know any other way of going about it.

By the way I did not know "dunce" is a regular word, this made me google it. You learn something every day...

LexiconCode commented 5 years ago

Are we in agreement @mrob95 @kendonB @comodoro to proceed with the following as a temporary measure?

Add back some rdescripts. The guiding rule being simplifying complex commands as demonstrated above or explaining ambiguity

comodoro commented 5 years ago

For me of course, that's what I was arguing for.

kendonB commented 5 years ago

The only place they actually get used is in the recognition window right? So whoever is saying the phrase will almost always know what it's for.

Once/if we're using rdescripts for documentation then it makes much more sense to have them when the command phrase does not describe the behavior.

So for now I only think it's worth it for those that come through the Natlink window looking like nonsense, which shouldn't be many.

mrob95 commented 5 years ago

Are we in agreement @mrob95 @kendonB @comodoro to proceed with the following as a temporary measure?

Add back some rdescripts. The guiding rule being simplifying complex commands as demonstrated above or explaining ambiguity

Given that they are only used in the status window, which I don't think many people are watching closely, I don't think it is a high priority at the moment. When/if we have a system for generating docs then the shape of the problem will be much better defined.

LexiconCode commented 4 years ago

@mrob95 I've re-factored apps and ccr in Caster 1.0.0 to share the same folder level directory. Since there's one grammar per file they've been grouped if an application has more than one grammar.

We have unified/consistent loading mechanisms across the user space and starter grammars.

Boilerplate has been reduced. I have templates already made for different types of grammars that can be dropped into the user directory to command to speed up grammar development. I have yet to implement copying them by voice.

Centralized imports have been reverted due to creating circular dependency loops and readability.

Functions have been separated from grammars into what is called support files. There are grammar.py files and grammar_support.py files.

I've also implemented further restructuring very similar to @kendonB recommendations. This will be slated for the next release after 1.0.0.

LexiconCode commented 4 years ago

Completed