[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
[dsdp-tm-dev] FTP Listing Parser Extension Point
|
Hello Javier,
thanks for moving the FTP Listing Parser extension point
from Services to the Subsystem. Seeing the code, I'm
actually more convinced than ever that this was the
right move -- the dropdown and extension point handling
are now all nicely in one plugin.
There is just few concerns I still have:
1. Avoid early plugin activation for extensions.
In FTPClientConfigFactory line 61, you do a
"createExectuableExtension". Because of that,
all plugins which contribute FTP parsers are
activated. This should be deferred to later.
Somebody contributing a VXWORKS parser should not
be activated if the VXWORKS parser is not chosen
or not applicable.
The class should only be loaded when the parser
is really used.
2. Translatable label of the FTP Parser extension.
According to your .exsd, the "name" attribute
is marked not translatable (although the description
says "displayed in the UI").
Please change the "name" attribute into an "id" attribute
which is used internally only; and add a "label" attribute
which is translatable and shown in the UI.
Only like that, we can
a) guarantee that no two extenders provide an extension
with exactly the same name -- they can use notation
like "com.windriver.ftp.parser.VXWORKS"
b) guarantee that in a multi-lingual team, if member A
has Eclipse translated to Swedish and member B to
English, they can still share the same configuration
c) guarantee that programs operating based on a well-
known ID of a parser always find the right parser
3. Autodetect.
As I have mentioned before, the AUTO special parser should
not be restricted to the preconfigured parsers only. User
contributed parsers should also be considered.
For doing that, I'd suggest that the extension has an
additional attribute "ftpSystemTypes" holding a regular
expression to be matched against the result of the FTP
SYST command. If multiple parsers match, they should be
tried one after the other. An RSEDelegatingFtpListingParser
could do this work.
See my previous E-Mail comments that I sent you below for reference.
Thanks,
--
Martin Oberhuber
Wind River Systems, Inc.
Target Management Project Lead, DSDP PMC Member
http://www.eclipse.org/dsdp/tm
________________________________
From: Oberhuber, Martin
Sent: Friday, April 20, 2007 6:10 PM
To: javier.montalvoorus@xxxxxxxxxxx
Cc: Target Management developer discussions
Subject: New FTP Listing Parser Extension Point
Hello Javier,
two more considerations:
* We should avoid having to activate plugins which contribute FTP
parsers, just in order to check whether they are valid. Therefore, the
extension point should have another attribute "systemNames" which should
contain a regular expression that needs to match the system name
reported by the remote side. If it matches, AUTO would activate the
plugin, instantiate the parser, and try that parser on that system.
Cheers,
Martin
Oberhuber, Martin schrieb:
Hello Javier,
I looked at your patch and it looks good to me.
I have no time right now to test it with VxWorks, but I'm
confident
that it will work fine. Your patch looks sizeable, so please go
ahead
and commit it.
What I would like to see going forward, is the ability that the
"AUTO" parser type also tries the various contributed parsers.
Right now, when I'm not mistaken, any extender-contributed
parser will only be used when explicitly selected by the user.
This is not user-friendly. We should allow extenders to
contribute
parsers, but the system should be able to select the contributed
parsers automatically wherever possible. In the simplest case,
the parser contribution could have an
boolean isValidFor(String systemType)
method that allows auto-selecting it based on the systemType
returned by the remote. In case multiple contributed parsers
return TRUE for isValidFor(), they could be tried one after the
other while parsing, until the first one is successful.
Thanks,
--
Martin Oberhuber
Wind River Systems, Inc.
Target Management Project Lead, DSDP PMC Member
http://www.eclipse.org/dsdp/tm <http://www.eclipse.org/dsdp/tm>
<http://www.eclipse.org/dsdp/tm>
--
Martin Oberhuber
Wind River Systems, Inc.
Target Management Project Lead, DSDP PMC Member
http://www.eclipse.org/dsdp/tm