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

Easier method for documenting and setting gestures for global plugin scripts #6266

Closed
ctoth opened this issue Aug 15, 2016 · 10 comments
Closed

Easier method for documenting and setting gestures for global plugin scripts #6266

ctoth opened this issue Aug 15, 2016 · 10 comments
Labels
Milestone

Comments

@ctoth
Copy link

@ctoth ctoth commented Aug 15, 2016

I propose and will implement the following:

class GlobalPlugin(GlobalPlugin):

    @document(__("""Short description in gesture editor"""))
    @gesture("kb:NVDA+shift+g")
    def script_do_something(self, gesture):
        pass #script contents here
@josephsl
Copy link
Collaborator

@josephsl josephsl commented Aug 15, 2016

@jcsteh
Copy link
Contributor

@jcsteh jcsteh commented Aug 16, 2016

In general, I like this idea. A few comments:

  1. Tiny correction: NVDA doesn't do lazy gettext at this stage, so __( should just be _(.

  2. We'd want support for multiple gestures. Perhaps it could take a list. If you want, it could take either a list or a string to make it easier for the majority case, though that does make it difficult to know whether to use gesture/gestures (singular/plural).

  3. Any reason not to combine this into one decorator?

      # Translators: Describes a command.
      @script(doc=_("Short desc"),
          gestures=["kb:NVDA+shift+g"])
      def script_foo(self, gesture):
          ...
    
  4. This should not be specific to global plugins. If you implement it on baseObject.ScriptableObject, it should work everywhere (tm).

@nvdaes
Copy link
Contributor

@nvdaes nvdaes commented Aug 16, 2016

I'm wondering if this can cover the assignment of sequential gestures
too, for instance: kb:NVDA`+g c, kb:NVDA+g d, as done for some add-ons
currently.
Also, should be script categories cover too? Thanks.

2016-08-16 2:55 GMT+02:00, James Teh notifications@github.com:

In general, I like this idea. A few comments:

  1. Tiny correction: NVDA doesn't do lazy gettext at this stage, so __(
    should just be _(.

  2. We'd want support for multiple gestures. Perhaps it could take a list. If
    you want, it could take either a list or a string to make it easier for the
    majority case, though that does make it difficult to know whether to use
    gesture/gestures (singular/plural).

  3. Any reason not to combine this into one decorator?

    # Translators: Describes a command.
    @script(doc=_("Short desc"),
        gestures=["kb:NVDA+shift+g"])
    def script_foo(self, gesture):
        ...
    
  4. This should not be specific to global plugins. If you implement it on
    baseObject.ScriptableObject, it should work everywhere (tm).

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#6266 (comment)

@camlorn
Copy link
Contributor

@camlorn camlorn commented Aug 17, 2016

I think it should also work on plane old functions.

I think that you can detect the difference with isinstance. As I discussed on IRC we should really move toward some sort of simpler, class-free, single-file script API. Unfortunately, I'd be willing to bet this needs some sort of changes in core to work.

@jcsteh
Copy link
Contributor

@jcsteh jcsteh commented Aug 17, 2016

@jcsteh
Copy link
Contributor

@jcsteh jcsteh commented Aug 17, 2016

I'm wondering if this can cover the assignment of sequential gestures
too, for instance: kb:NVDA`+g c, kb:NVDA+g d, as done for some add-ons
currently.

This is a different issue: #3687. Note that if that gets implemented, this should just work for the decorator being discussed here too (or at least it should be fairly easy to tweak).

Also, should be script categories cover too?

Definitely. if we go with the @script decorator, it can just be an additional keyword argument (category=SCRCAT_FOO).

@ctoth
Copy link
Author

@ctoth ctoth commented Aug 17, 2016

Currently categories appear to be set on the class level, So if you were
to define multiple @scripts it would set the last one as the category,
assuming we don't instead move the category attribute to the function
itself.
Should I leave this as is? Modify inputCore to check the function for a
scriptCategory attribute before the class? Something else entirely?

On 8/17/2016 4:26 PM, James Teh wrote:

I'm wondering if this can cover the assignment of sequential gestures
too, for instance: kb:NVDA`+g c, kb:NVDA+g d, as done for some add-ons
currently.

This is a different issue: #3687
#3687. Note that if that gets
implemented, this should just work for the decorator being discussed
here too (or at least it should be fairly easy to tweak).

Also, should be script categories cover too?

Definitely. if we go with the |@script| decorator, it can just be an
additional keyword argument (|category=SCRCAT_FOO|).


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#6266 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOcCHDSzgMReMHOlGFDthrmJ1sLjMJAks5qg4qhgaJpZM4Jk39r.

    -Q

Accessible Software for the Blind

http://GetAccessibleApps.com

@jcsteh
Copy link
Contributor

@jcsteh jcsteh commented Aug 17, 2016

@ctoth
Copy link
Author

@ctoth ctoth commented Aug 17, 2016

Woof. I just grepped for scriptCategory assuming it would be the same
attribute everywhere. @script will just set the .category of the method
then. I should have enough to make this happen now.

On 8/17/2016 4:33 PM, James Teh wrote:

There are two levels of category: class and method. For a method, there
is a .category attribute. See globalCommands for an example of where
this is used. The class category is used as the default if a method
doesn't explicitly specify a category.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#6266 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOcCFUyZREW70yIYMNBWsM1FJ6M0vi9ks5qg4wrgaJpZM4Jk39r.

    -Q

Accessible Software for the Blind

http://GetAccessibleApps.com

@nvdaes
Copy link
Contributor

@nvdaes nvdaes commented Aug 18, 2016

Thanks.

2016-08-18 0:26 GMT+02:00, James Teh notifications@github.com:

I'm wondering if this can cover the assignment of sequential gestures
too, for instance: kb:NVDA`+g c, kb:NVDA+g d, as done for some add-ons
currently.

This is a different issue: #3687. Note that if that gets implemented, this
should just work for the decorator being discussed here too (or at least it
should be fairly easy to tweak).

Also, should be script categories cover too?

Definitely. if we go with the @script decorator, it can just be an
additional keyword argument (category=SCRCAT_FOO).

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#6266 (comment)

ctoth pushed a commit to ctoth/nvda that referenced this issue Aug 18, 2016
…andler module

added a metaclass for collecting gestures on ScriptableObjects, this now supports getting from .gesture and .gestures set on script_ functions by the @script decorator.
Fixes nvaccess#6266
@leonardder leonardder added the feature label Feb 10, 2018
@nvaccessAuto nvaccessAuto modified the milestone: 2018.3 Jun 13, 2018
michaelDCurran added a commit that referenced this issue Jun 13, 2018
…ta (#8236)

* added an @script decorator for setting script metadata to the scriptHandler module
added a metaclass for collecting gestures on ScriptableObjects, this now supports getting from .gesture and .gestures set on script_ functions by the @script decorator.
Fixes #6266

* Removed gesture attribute from script functions
Now passing gesture to the ```@script``` decorator will append the gesture to the list of gestures, rather than setting both gesture and gestures attributes on the function.

* Removed unused getScripts method from BaseObject

* Address review comment and rename GestureCollector to ScriptableType

* Make the script decorator gestures work both for normal and dynamic NVDAObjects. Provide a working example for Excel

* First baceObject tests

* Fix decorated scripts not properly being added when subclassing objects

* Finish unit tests

* Add bypasInputHelp and resumeSayAllMode to the script decorator

* Review actions

* Add unit tests for the script decorator itself

* Update developer guide

* Review actions

* Review actions regarding devGuide and comments

* Only use the per class __gestures dictionary, also for decorated scripts

* Last review action

* Revert NVDAObjects/__init__.py to master, as in the end, this hasn't been touched by this change

* Also revert changes to test suite objectProvider

* Split of lines in devGuide

* Rename the scripts in test_baseObject to follow the phonetic alphabet, to fix a bug in baseObject that used only the first character of a script name.

* Log warnings when using the script decorator in a wrong way, either by decorating a non method or a method which name doesn't start with script_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.