[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: [RFC] Unified Application Scripting Interface
From:       ian reinhart geiser <geiseri () yahoo ! com>
Date:       2001-11-14 14:16:13
[Download RAW message or body]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wednesday 14 November 2001 05:30 am, Harri Porten wrote about Re: [RFC] 
Unified Application Scripting Interface:
> A few comments/questions about the code that I've see in the current CVS.
>
> a) Why is ScriptLoader limited to KMainWindow as parent type?
>    (counter example: that API makes it impossible to use it in
>    kword or any other koffice component)
>
> b) All the scripts currently seem to leak. Maybe setting the
>    autoDelete property of 'm_scripts' would be a good idea
>    (or much simpler: re-use QObject's parent-deletes-children-mechanism)
>
> c) Wouldn't it make sense to put the 'ScriptLoader' class into the
>    KDE namespace by K-prefixing' it?
>
	The scriptloader currently is nothing more than a testbed so that I can test 
the scriptinterface implementation.  Ideally it will be re-written to provide 
more features to manage the scripts.  This is a first attempt and I am in 
real need of assistance with createing a useful loader here.  The namespace 
makes perfect sense, along with the removing the parent object of 
KMainWindow.  As the week progresses I hope to have a more useful and 
flexable loader created.

Ideally  I want to split the class into two, a script manager and convenence 
class for adding scripts to the UI.  I was hopeing that as others looked at 
the code they would be able to provide input and this process could speed up.

> d) Why does ScriptInterface have a setParent method? Why not re-use
>    what you already have: QObject. Allocate the component
>    implementing the ScriptInterface with the desired parent object
>    as..erhm..parent object, as done through the factory?
>    (another idea, in combination with b) , would be to allocate the
>    components implementing the KScriptInterface with the script
>    loader as parent object and to provide a pair of 'setScriptParent' /
>    'scriptParent' -- maybe even the naming 'parent' is misleading
>    here)
>
	Yes in retrospect the name is misleading.  I got stuck because I could not 
think of a better name.  This fucntions/objects purpose that it will be the 
object that is shared between the C++ appliaction and the script engine.  
This will allow the jsembed and the python script engines to directly access 
the main applications objects without using dcop methods.   The reason this 
is not  done at the construction time is that the developer may want to 
dynamicly set what object is shared to the script engine.  If you have a 
suggestion for a better name please share.  My first names "ObjectTwin" and 
"ObjectProxy" seemed also very vuage.

> e) Minor API suggestion: What about renaming 'bool status()' to
>    'bool isRunning()' ? (or to change status() not to return a bool
>    but an enum providing more detailed information about the status of
>    the current script)
>
	yes, hmmm... i still dont why i did that...


> f) Why does KScriptInterface, as interface, inherit from QObject at
>    all?
>
	
	It should not, it should inherit from KPart::Plugin, it needs to be a 
Q_Object afaik so that the signals work.  Am I wrong here?  I find many of my 
impressions can sometimes be unfounded. :P

> g) Why does getScripts() (which lacks a const specifier btw :) )
>    return an action object? How about 'KAction *scriptAction()
>    const' along with an accessor to retrieve a list of QObjects that
>    implement the KScriptInterface?
>    (just an idea :)
>


	The idea behind this method is I wanted a dynamic way to add the script 
actions to the UI.  Since I did not want to dork with XML files I decided the 
SelectAction seemed the most correct.  It will allow the user to overview all 
of the Scripts in a menu and then select the one they wish to use.    Reall 
this is only an convinence method until the class is fixed.

> and isn't simply called scripts() ?

	Ah QT/KDE nameing vs Ian Geiser Part XXVX.  I am a firm beliver in get/set 
for accessors and mutatior.  And blah() type function names for static 
members.  Really this makes no difference to me, it is just style and I have 
sometimes forget to adhere.   I guess the most clear QT/KDE name for this 
class would be scriptActions(), yes?

	
>
> h) I suggest changing call functions accepting a string value to use a
> const reference (const QString& instead of QString).
>
	
	Yes, good idea.  I will do that.

> i) please rename KScriptInterface::Script() to lowercase script().
>

	Again naming... Ill conform soon, really I am trying :P

> j) is the errors(QString messages) meant to signal multiple errors ? The
> docu talks about "[a] current error message". Why not use a list of
> strings if the signature is right but the docu is wrong ?
>

	Docs are fight the signature is wrong.  I will change it to:
		void currentError(QStirng message);
		and
		void currentOutput(QString message);

> k) what is the error code passed by done(int errorCode) ? An arbritrary
> integer or an enum that could be defined in the header ?

	Currently it is return codes from KProcess or Python.  These are usually and 
int...  I guess I need to look at this more, the script engine may have to 
return a more meaningful string that can be set to a status bar or something.

I hope this answers some of your questions here, this is really a work in 
progress so I hope to see this take some real usability within the next week. 
 As trying as it is on ones ego sometimes peer review is the only way to get 
something like this to be safe and useful.   I will make the stated changes 
to the interface today and hopefully I will have a more logical script 
loader/manager done yet this week.

Thanks for the help.
- -ian reinhart geiser
- -- 
:-- Ian Reinhart Geiser --:
GPG Key: D6A6 7E16 13A9 B5A7 9E18 D1A7 3F2E B64D 19BC 76F8
===========================================================
"It's not just a computer -- it's your ass."
		-- Cal Keegan
===========================================================
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE78nytPy62TRm8dvgRAhxWAJ0ZNSSjL5RlIUHebG01QqkbkeIw+QCfQrwH
CFs+mTrJslJWyRC++NUhOnw=
=hhu6
-----END PGP SIGNATURE-----

[prev in list] [next in list] [prev in thread] [next in thread]