Gerd,
I agree that it is not ideal that these three properties (statement
separator / start of line comment / remove multi-line comments) are
displayed in the session properties as well as the plugin preferences
sheet (each plugin that defines a custom querytokenizer provides these
settings in that plugins preferences sheet). And beyond that, it is
also not ideal that changing the session properties sheet doesn't
affect the custom query tokenizer's settings. Perhaps a compromise
would be to apply the settings in session properties to a session's
query tokenizer. The main trouble that I see with this is that custom
tokenizer's are specific to a type of session (one with a plugin that
registers a custom tokenizer). But "New Session Properties" applies
globally to all "new" sessions, regardless of type.
I would like to illustrate this problem with an example. Suppose you
have three different aliases (Sybase, Oracle and Ingres). Now, you
also probably have installed the Sybase plugin, the Oracle plugin and
no plugin for Ingres (we don't have one). New Session Properties only
applies to the Ingres sessions, because the other two types of
sessions have a plugin which registers it's own tokenizer and keeps
it's own property values for the 3 properties (statement separator /
start of line comment / remove multi-line comments) and stored
procedure/function separator. The statement separator is the chief
culprit in making configuration a challenge for SQuirreL. On Oracle
it should be ";" for regular SQL statements and <eol> / <eol> for
stored procedures, functions and anonymous blocks. On Sybase, it
should be <eol> GO <eol> or <eol> go <eol> for all statement types.
For ingres, it is whatever you want, but probably ";" for regular
statements and go pick a character if you want to program stored
procedures - your life is not so easy with Ingres because it doesn't
have a plugin and therefore doesn't install a custom tokenizer where
you can keep settings for all of your scripts that make them work.
So, given the above setup, suppose a user launches squirrel and
accesses the "New Session Properties" dialog and configures ";" for
statement separator. Which session type(s) should this setting apply
to ? What will SQuirreL do to correctly parse stored procedures ?
I have to confess here, that I've never been a big fan of the dualism
we have for session properties (New Session Properties vs. This
Session's Properties) because it leads people again and again to
configure "This Session's Properties" and they think that the change
persists - that is until they launch a new session find out that it
doesn't. If we were going to make a change in favor of transparency,
I'd like to see if somehow we could eliminate "New Session Properties"
and have the "session properties" dialog be always persisted. I
suspect that we only needed a temporary "session-only" configuration
simply to support session deviation on the statement separator. Now
that we can provide much better support for properly parsing most
db-specific SQL that you are likely to want to run in SQuirreL, I
don't see the great necessity for "temporary session-only" properties.
Perhaps if we eliminate this distinction, we could also allow the
session properties to be populated from the custom tokenizer and also
write them back to the plugin preferences from the session properties
dialog. Going one step further would be to provide a plugin API to
allow custom session-specific properties to be registered in the
session properties dialog itself, potentially eliminating the need for
plugin preferences sheets altogether.
Sorry for the long email, I hope it is clear and let me know your thoughts.
Rob
On Sun, Mar 8, 2009 at 12:03 PM, Gerd Wagner <ger...@t-...> wrote:
> Hi all,
>
> finally I make it to come back to the point below.
>
> As far as I understand things now the concept of CustomQueryTokenizers goes
> quite a bit beyond the three simple Session Properties we had before. So
> simply replacing the properties in Session Properties may turn out
> inadequate.
>
> On the other hand I think it is very important that what the user sees and
> what a Plugin programmer uses should be made clear an transparent.
>
> This is to say I suggest we give programmers a unique interface to
> Tokenizers but also find a way to adequately display a custom tokenizers
> configs in our Session Properties. I'm not sure yet how far this has to go.
> The furtherest would be to provide custom tokenizer dependent panels.
>
> At the moment I think it is not such a nice thing that for some RDBMS we
> display props that aren't actually used.
>
> So far my 2 cents.
>
> Gerd
>
>
> Robert Manning wrote:
>>
>> Gerd,
>>
>> Is it necessary to override the user's preference for "remove
>> multiline comment" in HQLPanelController.onConvertToSQL (line 81) ? :
>>
>> String statementSeparator =
>> _sess.getProperties().getSQLStatementSeparator();
>> String startOfLineComment =
>> _sess.getProperties().getStartOfLineComment();
>> QueryTokenizer qt = new QueryTokenizer(statementSeparator,
>> startOfLineComment, true);
>> qt.setScriptToTokenize(hql);
>>
>> If it isn't then I'd propose we change the above to:
>>
>> QueryTokenizer qt = _sess.getQueryTokenizer();
>> qt.setScriptToTokenize(hql);
>>
>> It would be nice to keep the plugin overrides (via custom
>> QueryTokenizer) without transferring their values
>> to a Session's SessionProperties object. Less potential confusion and
>> Guido's recent fix in Session would
>> be unnecessary. I would also like to give SessionProperties an
>> interface that removes these methods to
>> users of Session so that they are forced (unless they cast) to use the
>> QueryTokenizer which should be
>> treated as the authority for these properties. If the override of
>> remove multi-line comment (the "true"
>> parameter above hard-codes remove multi-line comments to on,
>> regardless of what is in session
>> properties) is a hard requirement, then I would propose giving
>> IQueryTokenizer a clone method (or copy
>> constructor) and using that to get a new QueryTokenizer to hard-code
>> the value in. What are your
>> thoughts on this ?
>>
>> Rob
>>
>
>
|