@feerrenrut, @LeonarddeR could you take a look at this before I request an official review? I think this is something that should be discussed in general, since this issue is somehow blocked until NVDA supports the new style comments in MS Excel (see #9195).

Perhaps this can more easily be fixed if an appmodule can support different versions of an app.

I got a flake8 error, ET128 (flake8-tabs) unexpected number of tabs and spaces at start of call line (expected 2 tabs and 23 spaces, got 3 tabs and 0 spaces).

I think this specific error should be ignored since NVDA uses mostly tabs only in its code syntax.

I don't see a need to have the keystroke for reading a note be specific to Excel. Can't we use NVDA+alt+c for comments/notes/etc everywhere?

I'm going to mark this PR as a draft for now. Please mark it "ready for review" when the CI and review comments have been addressed. Thanks for your work on this @Adriani90

@feerrenrut regarding script decorator, my understanding was that this should be preferably used when addin gestures to input gestures dialog. These gestures are not in the input gestures dialog as far as I can see.

I've tried to convert some of them into script decorator, but I get a type error when I tried to convert the list of gestures for the script. changeSelection into script decorator for example.

I tried this:

	@script(
		gestures = ["kb:tab", "kb:shift+tab", "kb:enter", "kb:numpadEnter", "kb:shift+enter", "kb:shift+numpadEnter", "kb:upArrow", "kb:downArrow", "kb:leftArrow", "kb:rightArrow", "kb:control+upArrow", "kb:control+downArrow", "kb:control+leftArrow", "kb:control+rightArrow", "kb:home", "kb:end", "kb:control+home", "kb:control+end", "kb:shift+upArrow", "kb:shift+downArrow", "kb:shift+leftArrow", "kb:shift+rightArrow", "kb:shift+control+upArrow", "kb:shift+control+downArrow", "kb:shift+control+leftArrow", "kb:shift+control+rightArrow", "kb:shift+home", "kb:shift+end", "kb:shift+control+home", "kb:shift+control+end", "kb:shift+space", "kb:control+space", "kb:pageUp", "kb:pageDown", "kb:shift+pageUp", "kb:shift+pageDown", "kb:alt+pageUp", "kb:alt+pageDown", "kb:alt+shift+pageUp", "kb:alt+shift+pageDown", "kb:control+shift+8", "kb:control+pageUp", "kb:control+pageDown", "kb:control+a", "kb:control+v", "kb:shift+f11"]
	)
	def script_changeSelection(self,gestures):

I would be very grateful for some hint here ;)

@Adriani90 wrote:

I would be very grateful for some hint here ;)

You should wrote this:

def script_changeSelection(self, gesture):

not this:

def script_changeSelection(self,gestures):

The second parameter passed to each script is not a list of al keyboard shortcuts to which it is assigned but the object re[presenting a gesture (in this case a key press) which invoked it.

Thanks @lukaszgo1 for the clarification. I changed it, but there is still the same error,

  File "NVDAObjects\window\excel.py", line 446, in <module>
    class ExcelBrowseModeTreeInterceptor(browseMode.BrowseModeTreeInterceptor):
  File "baseObject.py", line 179, in __new__
    gestures[gesture] = scriptName
TypeError: unhashable type: 'list'

I have the feeling that this should be very trivial to fix, but I as a spare time python beginner am still struggling with simple things like this ;)

Sorry for not noticing it when reading for the first time - I've assumed you've imported script from scriptHandler but it seems you haven't. You should decorate this method as follows:
@scriptHandler.script(...

Also when listing all keyboard shortcuts to which this script is assigned it is a good idea to use tuple rather than list - as we do not need mutability in this case and tuple uses less memory.

Thanks for the good tipp, I converted all the script decorators to touples, this solved the error. It seems it's because in Python 3 lists are not hashable, or at least this is how I understood it.

Overall converting to script decorator saved almost 20 lines in that file.

I tested in MS Excel 365 and 2010, all key strokes are working as expected and all messages are reported correctly (i.e. not on a note, cell x set as column header, etc.).

@Adriani90 I hope you are aware that you can run the lint check locally scons lint base=origin/master. It is much faster than pushing and waiting for an appveyor build.

@feerrenrut thank you very much for the suggestion. Is there a possibility to specify in which file the lint should be performed? When I do base=origine/master it fetches all files and this takes very long time.

You should do lint base=master not origin/master. Alternatively you should synchronize your remote master with the NV Access copy.

So finally all the actions should be addressed now, the following problem still remains:

1. With this PR, "Has notes" is also announced in earlier versions of MS Office. We need two separate controltypes in controltypes.py, one which announces "has comment" in MS Excel 2013 and earlier (can stay as it is) and one which announces "has note" in MS Excel 2016 and newer (a new one only for MS Excel).
2. We need a new controltype in controltypes.py for MS Excel which announces "has comment" for new style comments (see NVDA does not report (new style) comments in Excel 365 #9195).

For the first problem, is it possible to copy the current controltype which is used and specify another label to it and say that this should be only used for MS Excel 2016 and newer?