[Python] Adds keyboard to dialog class, using all of Dialog().numeric() methods.

[nuka1195]

This does not eliminate xbmc.Keyboard() or Dialog().numeric. Those could be depreciated.

[nuka1195]

@MartijnKaijser I don't see anyway other than modifying the base class to enter date other than DD/MM/YYYY

My rebase eliminated all comments.

The changes to GUIDialogGamePad and GUIDialogNumeric were necessary to not return Hashed values.

[nuka1195]

I originally returned the hashed result, which might make more sense. It is a password keyboard.

[nuka1195]

I eliminated the changes to GUIDialogGamePad and GUIDialogNumeric and just return the hashed password.

[nuka1195]

Is this what you had in mind?

[nuka1195]

And is there a way to rebase without losing someone's comments?

[nuka1195]

Also #2767 needs to be considered otherwise all pydocs that list "default" as a keyword are wrong as currently the keyword would be "defaultt"

[jmarshallnz]

Rebase always kills off comments (as they're assigned to the old commits) - you can get to them if you know the hash still (i.e. via your email).

That is exactly what I was suggesting. A couple of minor points:

1. You could use defaultText to get around the defaulttt issue. Or just text, currentText etc.
2. I'm not sure if QWERTY is best here given many keyboards are not querty. Perhaps ALPHANUM ?
3. I'm guessing that hidden input (applying only to qwerty input) is if you want a plain text password to be entered for some other transformation. It might be useful to note in the doxy that password input is always hidden?

Looks fine other than that.

[nuka1195]

1. Sure those are good. If no current addons are broken by defaultt, then that would be better than defaultt.
2. Will change.
3. Yes that was the reasoning. Will add a note.

About your comment getting to the numeric keyboard from the standard keyboard. I only see IP keyboard as a shortcut. Maybe I should add that one back in, but leave it only up to the developer. No choose.

[jmarshallnz]

I'm not sure why current addons would be broken - this is a new function, right? Or are you meaning there'd be inconsistency between the specific dialog methods and this generic one?

Regarding the numeric, what I meant (but didn't say!) was that you can punch the numbers in on the keyboard slightly less efficiently than the number. I'm not sure what use-cases there are where you know that there's a numeric password and want it hashed.

[nuka1195]

I meant other methods that had default as a parameter, if they used the
keyword which I think you would have had reports if it had. I like using
keywords to quickly know what I'm passing especially for methods that the
order of parameters was odd.

I'll make the changes you requested, then maybe a discussion on making all
the methods more consistent.

[nuka1195]

minor pydocs fixups and some unneeded include's

[jimfcarroll]

I hate to be annoying (well ... usually), and if it matters to anyone, but since the 'type' is an 'int' it should probably use a 'switch' statement (which is more efficient) than an if/else chain.

Plus, just a style note, the "get" statements on the module are really just a means of exporting the constants. I would use the defines in C++ code rather than calling the getters. If you do the above (change to a switch statement) you'll be forced to anyway.

[nuka1195]

I agree, but I was having difficulty getting the defines to work right and have them set as constants in xbmcgui module.

Where should these defines be put? outside python? That's why I ended up just using the getters in this and notification dialog. Also why I didn't use a switch.

Any hints?

[jmarshallnz]

You'll need to move the defines to somewhere that you have access to from Dialog.cpp and ModuleXbmcGui.cpp. i.e. a header.

[jimfcarroll]

Maybe Dialog.h. Then include that in ModuleXbmcGui.cpp (as long as there's no circular includes).

[nuka1195]

Right, I put them in dialog.h before I think and had issues. Maybe that was before trying the getters for setting constants.

I'll play with it. If I get it to work do you want a patch for notification which also uses the getters?

[nuka1195]

I can not get this to work.

PyModule_AddStringConstant(module,"NOTIFICATION_INFO",getNOTIFICATION_INFO());
PyModule_AddStringConstant(module,"NOTIFICATION_WARNING",getNOTIFICATION_WARNING());
PyModule_AddStringConstant(module,"NOTIFICATION_ERROR",getNOTIFICATION_ERROR());
PyModule_AddIntConstant(module,"CONTROL_TEXT_OFFSET_X",10);
PyModule_AddIntConstant(module,"CONTROL_TEXT_OFFSET_Y",2);
PyModule_AddStringConstant(module,"NOTIFICATION_INFO",info);
PyModule_AddStringConstant(module,"NOTIFICATION_WARNING",warning);
PyModule_AddStringConstant(module,"NOTIFICATION_ERROR",error);

notice the last three duplicated lines in generated AddonModuleXbmcgui.cpp. if you could have a look at the following to see what I'm doing wrong?

nuka1195@af8c5a2

This was a simple test to see if I could get it to work. This was the issue i ran into before.

[nuka1195]

Also I notice Dialog.h get's included twice in AddonModuleXbmcgui.cpp, is this a circular reference issue?

#include "interfaces/legacy/Dialog.h"
#include "interfaces/legacy/ModuleXbmcgui.h"
#include "interfaces/legacy/Control.h"
#include "interfaces/legacy/Window.h"
#include "interfaces/legacy/WindowDialog.h"
#include "interfaces/legacy/Dialog.h"
#include "interfaces/legacy/WindowXML.h"

[jimfcarroll]

Discussion over here: nuka1195@af8c5a2#commitcomment-3350583

Including Dialog.h twice isn't a circular dependency but is also unnecessary.

[jimfcarroll]

@jmarshallnz , I don't really care if my suggestions go in. It's fine the way it is now if you want to take it. It was just a suggestion but it's not important. I'll let you push it as you see fit.

[nuka1195]

So the issue is with SWIG? It would have been nice to put the defines in dialog.h, I just couldn't get it to work.

[jimfcarroll]

The issue is with String constants in the codegenerator. Which is why I only wanted you to move the constants you added as part of this commit (which are all ints).

[nuka1195]

Then I'll move them. @jmarshallnz please wait til my next push. I'll post when I'm done.

[nuka1195]

@jimfcarroll, It's still generating two constants for the defines i put in Dialog.h. it compiles and links in OS X.

PyModule_AddIntConstant(module,"INPUT_ALPHANUM",INPUT_ALPHANUM);
PyModule_AddIntConstant(module,"INPUT_NUMERIC",INPUT_NUMERIC);
PyModule_AddIntConstant(module,"INPUT_DATE",INPUT_DATE);
PyModule_AddIntConstant(module,"INPUT_TIME",INPUT_TIME);
PyModule_AddIntConstant(module,"INPUT_IPADDRESS",INPUT_IPADDRESS);
PyModule_AddIntConstant(module,"INPUT_PASSWORD",INPUT_PASSWORD);
PyModule_AddIntConstant(module,"PASSWORD_VERIFY",PASSWORD_VERIFY);
PyModule_AddIntConstant(module,"ALPHANUM_HIDE_INPUT",ALPHANUM_HIDE_INPUT);

[these two left so maybe it's a hint to you as order of creation]
PyModule_AddIntConstant(module,"CONTROL_TEXT_OFFSET_X",10);
PyModule_AddIntConstant(module,"CONTROL_TEXT_OFFSET_Y",2);

PyModule_AddIntConstant(module,"INPUT_ALPHANUM",0);
PyModule_AddIntConstant(module,"INPUT_NUMERIC",1);
PyModule_AddIntConstant(module,"INPUT_DATE",2);
PyModule_AddIntConstant(module,"INPUT_TIME",3);
PyModule_AddIntConstant(module,"INPUT_IPADDRESS",4);
PyModule_AddIntConstant(module,"INPUT_PASSWORD",5);
PyModule_AddIntConstant(module,"PASSWORD_VERIFY",1);
PyModule_AddIntConstant(module,"ALPHANUM_HIDE_INPUT",2);

[nuka1195]

actually that doesn't compile now.

[nuka1195]

ok this does compile

[nuka1195]

not sure what your decision is on this, since it adds two constants for each define when done this way, but i rebased with master so you wouldn't get any merge conflicts.

[jimfcarroll]

@jmarshallnz, I'm fine with this. My suggestion was only minor and when I get some time maybe I'll play around with it a little. In my absence (since my participation is sparse right now) Can you be the final approver of the Python stuff? I watch the emails and try to review things Python related but I'll let you push the button during the windows if that's OK with you? Thanks.