[sip-comm-dev] Keybinding Chooser Addition


#1

Hello Damian,

It looks like there are a few problems now with the new keybinding plugin. Here are a few I've noticed that worked before, but now don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side. Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

···

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of the plugin.keybindingchooser package into impl.keybindings.

Ack! Sorry, I misread your original comment and thought the proposition was to do away with the service. My bad.

I was trying to say that since the configuration UI is quite tightly coupled with this service implementation and both are quite light then they probably should belong to the same package.... It would have made sense to keep them separated if there's a chance of someone needing concurrent versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.

Well unless I am missing something we'll probably only need what's in the chooser package which gives us approximately 1400 lines of code. The main reason I am insisting on this is that we'll want to modify the UI and having the code in there would make it easier... If you don't feel like making the change then, by all means, just leave it to us. Your contribution is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then this makes sense, and the 'alternative bindings' are certainly substantial enough. I'm sorry if I gave the impression that I wasn't willing to make the change- I'll handle it.

... I meant alternative bindings

I've never seen this feature before but should be simple once the chooser's source is added in.

Hopefully we'll soon be able to start integrating the spell checker soon too. :slight_smile:

Cheers,
Damian

PS. I give up on going by 'Galen'. With webmail metadata adding in my name this is just confusing. Oh well- it was worth a try...

On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Damian,

Damian Johnson написа:

    First, I don't quite see the need of splitting the implementation
    into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins to
be able to listen for and process keyboard events?

Most probably.

If so then the
addition shouldn't use the new methods provided via the SIPCommFrame. If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should belong
to the same package.

    The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as a
plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

    It would also be nice if we could copy necessary classes from your
    lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor to
easily tweak the keybindings. If integrated we'd still probably need the
jar for that functionality (with the exception of the Persistence class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet (providing
an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

    Then, storing default and user preferences for key bindings in a
    binary form makes it difficult to change them for no real reason.
    I've just seen that your lib already allows to store settings in a
    properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite often
and not being able to do so by editing a text file would be a problem.

The
advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by manually
adding indexes. Here's a very rough example:

....
<impl>
   <keybindings>
       <binding1>
           <actionName value="Previous Tab"/>
           ...
       </binding1>
       <binding2>
           <actionName value="Next Tab"/>
           ...
       </binding2>
   </keybindings>
</impl>

    We can put the default ones into the defaults.properties files and
    then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look
into it.

Yes the defaults are new indeed.

    1. Would it be possible to implement support for alternative
    actions? Most key binding management UIs have it would be really
    nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!

Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,
Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org > <mailto:emcho@sip-communicator.org>> wrote:

    Hey Galen,

    Nice work indeed and the feature is quite useful. I've just tried it and
    I like it a lot. Thanks for contributing!

    There are however a few things that make me feel uncomfortable with the
    way this is currently integrated in SIP Communicator.

    First, I don't quite see the need of splitting the implementation into a
    service and a plugin. The configuration UI could very well live in
    impl.keybinding. It would also be nice if we could copy necessary
    classes from your lib into the service implementation. There are two
    reasons I believe this would a good idea:
           - We'd probably need to make changes on some parts such as the
    configuration UI and the persistence mechanisms (see below) so that they
    would fit the SC conventions.
           - It would make the code easier to understand.

    Then, storing default and user preferences for key bindings in a binary
    form makes it difficult to change them for no real reason. I've just
    seen that your lib already allows to store settings in a properties file
    so I think this would fit better. We can put the default ones into the
    defaults.properties files and then use the configuration service to
    modify them.

    What do you think?

    Other than that I was wondering if I could make the following feature
    requests:

    1.Would it be possible to implement support for alternative actions?
    Most key binding management UIs have it would be really nice if we
    also did.
    and
    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

    Once again, thanks for contributing your work!

    Emil

    Yana Stamcheva написа:
    > Hi Galen,
    >
    > you've done a really great job in this patch!
    >
    > First let me apologize for making you wait so long, we were really
    busy
    > lately. We have discussed the plugin off list already, but it's now
    > applied and committed. You're also on our contributors page.
    >
    > (more inline)
    >
    > Damian Johnson wrote:
    >> Hi. I've just finished integrating a chooser for keybindings into
    the SIP
    >> Communicator. This both provides a means for users to change their
    >> keybinding preferences and save them between runs. Most of the
    details of
    >> the addition are described in the latest blog post
    >> here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
    >> two additions:
    >> 1. Keybinding persistence via a new service. This saves custom
    bindings with
    >> the user's preferences and informs any frames using the bindings
    of updates.
    >> *src/net/java/sip/communicator/impl/keybindings
    >> src/net/java/sip/communicator/service/keybindings
    >
    > Applied. I like the way you have implemented key bindings in the
    gui bundle!
    >
    >> *
    >> 2. Plugin that provides a ConfigurationForm for changing the
    keybindings.
    >> *src/net/java/sip/communicator/plugin/keybindingChooser*
    >>
    >> I've added a how-to for adding new keybinding sets
    >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
    >> chooser description). Also, this is my first time making a patch
    >> - are binaries supposed to be provided separately like this? I
    didn't spot
    >> an option to include them in svn diff...
    >
    > My reply is coming late, but yes binaries should be supplied
    separately,
    > because they're not contained in the diff. The how-to is also very
    good
    > idea. Let me know if you're interested in adding this on the SIP
    > Communicator website.
    >
    >> A couple of usability features (any opinions?):
    >> 1. Add a key to revert bindings to defaults. Helpful or unnecessary?
    >> 2. Keybindings aren't changed until the user hits "Apply".
    Perhaps bindings
    >> that haven't been applied should be discolored to draw attention
    to them?
    >
    > My personal opinion is that a button "Defaults" could be really
    helpful.
    > Otherwise coloring unsaved bindings could be useful, but also it could
    > be confusing, the user could not be sure what exactly the color means.
    >
    >> Known Issue:
    >> Not all permissible bindings work in all situations. For instance
    in the
    >> main frame the 'Contacts' and 'Chat rooms' panels appear to
    intercept some
    >> keyboard events, preventing bindings to directional arrows (without
    >> modifiers) or the space bar from working. This isn't a terribly
    big woop but
    >> might possibly confuse users.
    >
    > Hm, could you explain more in details the problem situations, we could
    > try to resolve this.
    >
    >> A few unrelated things I've come across:
    >> 1. The patch also includes the fix for a minor (but very consistent)
    >> spelling error: 'desactivate' to 'deactivate'. This included
    tweaking an
    >> image with the word, changing some language files, and
    refactoring parts of
    >> the ManageButtonsPanel class.
    >
    > Committed all spelling corrections. Thanks.
    >
    >> 2. Does anyone know if there's a known issue with the chatalerter
    plugin?
    >> Eclipse has complained that the org.jdesktop.jdic.misc package
    can't be
    >> resolved ever since I checked it out via svn. However this has never
    >> prevented the SIP Communicator from running. I've tried asking a
    couple of
    >> times on the IRC channel but I think I keep catching everyone
    when they're
    >> asleep...
    >
    > Maybe you've already figured it out yourself, but I'm just
    replying for
    > the record..Even that Eclipse is complaining about a library, if you
    > compile and run SIP Communicator through Ant it will always work,
    > because the classpath is set inside the build.xml. If Eclipse is
    > complaining about a library this means that this library is not
    added in
    > the .classpath file of Eclipse. You need to do the following: right
    > click on the project / "Properties" / "Project Build Path" /
    "Libraries"
    > / "Add new" / Find the library in the lib directory of SComm.
    >
    >> 3. Couple of minor issues on the site:
    >> Developer Documentation > How to Configure Eclipse
    >> Minor simplification- it's not necessary to get the old version
    of Subclipse
    >> then upgrade it. The buckminster error can be avoided by
    disabling the
    >> optional components.
    >
    > Nice catch! I didn't experienced this error, so I'm not very familiar
    > with the problem, so again are you interested in changing the part of
    > the tutorial concerning the problem.
    >
    >> Developer Documentation > UI Service > Create and add
    configuration forms
    >> As Atul noticed the 'getConfigurationManager()' method doesn't
    exist and
    >> should be 'getConfigurationWindow()'.
    >
    > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
    this one,
    > could you be more precise?
    >
    > Damian, the patch was very well documented, very mature and easy to
    > read. Again you've done a very good work! Thank you for the
    patience and
    > keep up the good work:)
    >
    > Cheers,
    > Yana
    >
    >> Hope this helps. Cheers! -Galen
    >>
    >> PS. I'm planning to try going by my middle name, Galen, in SIP
    >> correspondence to avoid confusion with Damien and the other
    Damian. :slight_smile:
    >>
    >>
    >>
    >>
    ------------------------------------------------------------------------
    >>
    >>
    >>
    ------------------------------------------------------------------------
    >>
    >>
    >>
    ------------------------------------------------------------------------
    >>
    >> ---------------------------------------------------------------------
    >> To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

    >
    > ---------------------------------------------------------------------
    > To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

    >
    >

    ---------------------------------------------------------------------
    To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work, but I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when sending/receiving messages ? The first thing that comes to me is that it could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

···

Hello Damian,

It looks like there are a few problems now with the new keybinding plugin. Here are a few I've noticed that worked before, but now don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side. Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of the plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the proposition was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly coupled with this service implementation and both are quite light then they probably should belong to the same package.... It would have made sense to keep them separated if there's a chance of someone needing concurrent versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's in the chooser package which gives us approximately 1400 lines of code. The main reason I am insisting on this is that we'll want to modify the UI and having the code in there would make it easier... If you don't feel like making the change then, by all means, just leave it to us. Your contribution is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then this makes sense, and the 'alternative bindings' are certainly substantial enough. I'm sorry if I gave the impression that I wasn't willing to make the change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker soon too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in my name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Damian,

Damian Johnson ������:

    First, I don't quite see the need of splitting the implementation
    into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins to
be able to listen for and process keyboard events?

Most probably.

If so then the
addition shouldn't use the new methods provided via the SIPCommFrame. If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should belong
to the same package.

    The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as a
plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

    It would also be nice if we could copy necessary classes from your
    lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor to
easily tweak the keybindings. If integrated we'd still probably need the
jar for that functionality (with the exception of the Persistence class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet (providing
an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

    Then, storing default and user preferences for key bindings in a
    binary form makes it difficult to change them for no real reason.
    I've just seen that your lib already allows to store settings in a
    properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite often
and not being able to do so by editing a text file would be a problem.

The
advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by manually
adding indexes. Here's a very rough example:

....
<impl>
   <keybindings>
       <binding1>
           <actionName value="Previous Tab"/>
           ...
       </binding1>
       <binding2>
           <actionName value="Next Tab"/>
           ...
       </binding2>
   </keybindings>
</impl>

    We can put the default ones into the defaults.properties files and
    then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look
into it.

Yes the defaults are new indeed.

    1. Would it be possible to implement support for alternative
    actions? Most key binding management UIs have it would be really
    nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!

Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,
Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org > >> <mailto:emcho@sip-communicator.org>> wrote:

    Hey Galen,

    Nice work indeed and the feature is quite useful. I've just tried it and
    I like it a lot. Thanks for contributing!

    There are however a few things that make me feel uncomfortable with the
    way this is currently integrated in SIP Communicator.

    First, I don't quite see the need of splitting the implementation into a
    service and a plugin. The configuration UI could very well live in
    impl.keybinding. It would also be nice if we could copy necessary
    classes from your lib into the service implementation. There are two
    reasons I believe this would a good idea:
           - We'd probably need to make changes on some parts such as the
    configuration UI and the persistence mechanisms (see below) so that they
    would fit the SC conventions.
           - It would make the code easier to understand.

    Then, storing default and user preferences for key bindings in a binary
    form makes it difficult to change them for no real reason. I've just
    seen that your lib already allows to store settings in a properties file
    so I think this would fit better. We can put the default ones into the
    defaults.properties files and then use the configuration service to
    modify them.

    What do you think?

    Other than that I was wondering if I could make the following feature
    requests:

    1.Would it be possible to implement support for alternative actions?
    Most key binding management UIs have it would be really nice if we
    also did.
    and
    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

    Once again, thanks for contributing your work!

    Emil

    Yana Stamcheva ������:
    > Hi Galen,
    >
    > you've done a really great job in this patch!
    >
    > First let me apologize for making you wait so long, we were really
    busy
    > lately. We have discussed the plugin off list already, but it's now
    > applied and committed. You're also on our contributors page.
    >
    > (more inline)
    >
    > Damian Johnson wrote:
    >> Hi. I've just finished integrating a chooser for keybindings into
    the SIP
    >> Communicator. This both provides a means for users to change their
    >> keybinding preferences and save them between runs. Most of the
    details of
    >> the addition are described in the latest blog post
    >> here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
    >> two additions:
    >> 1. Keybinding persistence via a new service. This saves custom
    bindings with
    >> the user's preferences and informs any frames using the bindings
    of updates.
    >> *src/net/java/sip/communicator/impl/keybindings
    >> src/net/java/sip/communicator/service/keybindings
    >
    > Applied. I like the way you have implemented key bindings in the
    gui bundle!
    >
    >> *
    >> 2. Plugin that provides a ConfigurationForm for changing the
    keybindings.
    >> *src/net/java/sip/communicator/plugin/keybindingChooser*
    >>
    >> I've added a how-to for adding new keybinding sets
    >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
    >> chooser description). Also, this is my first time making a patch
    >> - are binaries supposed to be provided separately like this? I
    didn't spot
    >> an option to include them in svn diff...
    >
    > My reply is coming late, but yes binaries should be supplied
    separately,
    > because they're not contained in the diff. The how-to is also very
    good
    > idea. Let me know if you're interested in adding this on the SIP
    > Communicator website.
    >
    >> A couple of usability features (any opinions?):
    >> 1. Add a key to revert bindings to defaults. Helpful or unnecessary?
    >> 2. Keybindings aren't changed until the user hits "Apply".
    Perhaps bindings
    >> that haven't been applied should be discolored to draw attention
    to them?
    >
    > My personal opinion is that a button "Defaults" could be really
    helpful.
    > Otherwise coloring unsaved bindings could be useful, but also it could
    > be confusing, the user could not be sure what exactly the color means.
    >
    >> Known Issue:
    >> Not all permissible bindings work in all situations. For instance
    in the
    >> main frame the 'Contacts' and 'Chat rooms' panels appear to
    intercept some
    >> keyboard events, preventing bindings to directional arrows (without
    >> modifiers) or the space bar from working. This isn't a terribly
    big woop but
    >> might possibly confuse users.
    >
    > Hm, could you explain more in details the problem situations, we could
    > try to resolve this.
    >
    >> A few unrelated things I've come across:
    >> 1. The patch also includes the fix for a minor (but very consistent)
    >> spelling error: 'desactivate' to 'deactivate'. This included
    tweaking an
    >> image with the word, changing some language files, and
    refactoring parts of
    >> the ManageButtonsPanel class.
    >
    > Committed all spelling corrections. Thanks.
    >
    >> 2. Does anyone know if there's a known issue with the chatalerter
    plugin?
    >> Eclipse has complained that the org.jdesktop.jdic.misc package
    can't be
    >> resolved ever since I checked it out via svn. However this has never
    >> prevented the SIP Communicator from running. I've tried asking a
    couple of
    >> times on the IRC channel but I think I keep catching everyone
    when they're
    >> asleep...
    >
    > Maybe you've already figured it out yourself, but I'm just
    replying for
    > the record..Even that Eclipse is complaining about a library, if you
    > compile and run SIP Communicator through Ant it will always work,
    > because the classpath is set inside the build.xml. If Eclipse is
    > complaining about a library this means that this library is not
    added in
    > the .classpath file of Eclipse. You need to do the following: right
    > click on the project / "Properties" / "Project Build Path" /
    "Libraries"
    > / "Add new" / Find the library in the lib directory of SComm.
    >
    >> 3. Couple of minor issues on the site:
    >> Developer Documentation > How to Configure Eclipse
    >> Minor simplification- it's not necessary to get the old version
    of Subclipse
    >> then upgrade it. The buckminster error can be avoided by
    disabling the
    >> optional components.
    >
    > Nice catch! I didn't experienced this error, so I'm not very familiar
    > with the problem, so again are you interested in changing the part of
    > the tutorial concerning the problem.
    >
    >> Developer Documentation > UI Service > Create and add
    configuration forms
    >> As Atul noticed the 'getConfigurationManager()' method doesn't
    exist and
    >> should be 'getConfigurationWindow()'.
    >
    > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
    this one,
    > could you be more precise?
    >
    > Damian, the patch was very well documented, very mature and easy to
    > read. Again you've done a very good work! Thank you for the
    patience and
    > keep up the good work:)
    >
    > Cheers,
    > Yana
    >
    >> Hope this helps. Cheers! -Galen
    >>
    >> PS. I'm planning to try going by my middle name, Galen, in SIP
    >> correspondence to avoid confusion with Damien and the other
    Damian. :slight_smile:
    >>
    ------------------------------------------------------------------------
    >>
    ------------------------------------------------------------------------
    >>
    ------------------------------------------------------------------------
    >>
    >> ---------------------------------------------------------------------
    >> To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

    >
    > ---------------------------------------------------------------------
    > To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

    >

    ---------------------------------------------------------------------
    To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hi Pavel,

I have committed a fix and the font button should be now working (from build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

···

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work, but I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when sending/receiving messages ? The first thing that comes to me is that it could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding plugin. Here are a few I've noticed that worked before, but now don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side. Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of the plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the proposition was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly coupled with this service implementation and both are quite light then they probably should belong to the same package.... It would have made sense to keep them separated if there's a chance of someone needing concurrent versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's in the chooser package which gives us approximately 1400 lines of code. The main reason I am insisting on this is that we'll want to modify the UI and having the code in there would make it easier... If you don't feel like making the change then, by all means, just leave it to us. Your contribution is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then this makes sense, and the 'alternative bindings' are certainly substantial enough. I'm sorry if I gave the impression that I wasn't willing to make the change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker soon too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in my name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov >> <emcho@sip-communicator.org> wrote:

Hey Damian,

Damian Johnson ������:

    First, I don't quite see the need of splitting the implementation
    into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins to
be able to listen for and process keyboard events?

Most probably.

If so then the
addition shouldn't use the new methods provided via the SIPCommFrame. If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should belong
to the same package.

    The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as a
plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

    It would also be nice if we could copy necessary classes from your
    lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor to
easily tweak the keybindings. If integrated we'd still probably need the
jar for that functionality (with the exception of the Persistence class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet (providing
an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

    Then, storing default and user preferences for key bindings in a
    binary form makes it difficult to change them for no real reason.
    I've just seen that your lib already allows to store settings in a
    properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite often
and not being able to do so by editing a text file would be a problem.

The
advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by manually
adding indexes. Here's a very rough example:

....
<impl>
   <keybindings>
       <binding1>
           <actionName value="Previous Tab"/>
           ...
       </binding1>
       <binding2>
           <actionName value="Next Tab"/>
           ...
       </binding2>
   </keybindings>
</impl>

    We can put the default ones into the defaults.properties files and
    then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look
into it.

Yes the defaults are new indeed.

    1. Would it be possible to implement support for alternative
    actions? Most key binding management UIs have it would be really
    nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!

Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,
Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org >> >>> <mailto:emcho@sip-communicator.org>> wrote:

    Hey Galen,

    Nice work indeed and the feature is quite useful. I've just tried it and
    I like it a lot. Thanks for contributing!

    There are however a few things that make me feel uncomfortable with the
    way this is currently integrated in SIP Communicator.

    First, I don't quite see the need of splitting the implementation into a
    service and a plugin. The configuration UI could very well live in
    impl.keybinding. It would also be nice if we could copy necessary
    classes from your lib into the service implementation. There are two
    reasons I believe this would a good idea:
           - We'd probably need to make changes on some parts such as the
    configuration UI and the persistence mechanisms (see below) so that they
    would fit the SC conventions.
           - It would make the code easier to understand.

    Then, storing default and user preferences for key bindings in a binary
    form makes it difficult to change them for no real reason. I've just
    seen that your lib already allows to store settings in a properties file
    so I think this would fit better. We can put the default ones into the
    defaults.properties files and then use the configuration service to
    modify them.

    What do you think?

    Other than that I was wondering if I could make the following feature
    requests:

    1.Would it be possible to implement support for alternative actions?
    Most key binding management UIs have it would be really nice if we
    also did.
    and
    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

    Once again, thanks for contributing your work!

    Emil

    Yana Stamcheva ������:
    > Hi Galen,
    >
    > you've done a really great job in this patch!
    >
    > First let me apologize for making you wait so long, we were really
    busy
    > lately. We have discussed the plugin off list already, but it's now
    > applied and committed. You're also on our contributors page.
    >
    > (more inline)
    >
    > Damian Johnson wrote:
    >> Hi. I've just finished integrating a chooser for keybindings into
    the SIP
    >> Communicator. This both provides a means for users to change their
    >> keybinding preferences and save them between runs. Most of the
    details of
    >> the addition are described in the latest blog post
    >> here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
    >> two additions:
    >> 1. Keybinding persistence via a new service. This saves custom
    bindings with
    >> the user's preferences and informs any frames using the bindings
    of updates.
    >> *src/net/java/sip/communicator/impl/keybindings
    >> src/net/java/sip/communicator/service/keybindings
    >
    > Applied. I like the way you have implemented key bindings in the
    gui bundle!
    >
    >> *
    >> 2. Plugin that provides a ConfigurationForm for changing the
    keybindings.
    >> *src/net/java/sip/communicator/plugin/keybindingChooser*
    >>
    >> I've added a how-to for adding new keybinding sets
    >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
    >> chooser description). Also, this is my first time making a patch
    >> - are binaries supposed to be provided separately like this? I
    didn't spot
    >> an option to include them in svn diff...
    >
    > My reply is coming late, but yes binaries should be supplied
    separately,
    > because they're not contained in the diff. The how-to is also very
    good
    > idea. Let me know if you're interested in adding this on the SIP
    > Communicator website.
    >
    >> A couple of usability features (any opinions?):
    >> 1. Add a key to revert bindings to defaults. Helpful or unnecessary?
    >> 2. Keybindings aren't changed until the user hits "Apply".
    Perhaps bindings
    >> that haven't been applied should be discolored to draw attention
    to them?
    >
    > My personal opinion is that a button "Defaults" could be really
    helpful.
    > Otherwise coloring unsaved bindings could be useful, but also it could
    > be confusing, the user could not be sure what exactly the color means.
    >
    >> Known Issue:
    >> Not all permissible bindings work in all situations. For instance
    in the
    >> main frame the 'Contacts' and 'Chat rooms' panels appear to
    intercept some
    >> keyboard events, preventing bindings to directional arrows (without
    >> modifiers) or the space bar from working. This isn't a terribly
    big woop but
    >> might possibly confuse users.
    >
    > Hm, could you explain more in details the problem situations, we could
    > try to resolve this.
    >
    >> A few unrelated things I've come across:
    >> 1. The patch also includes the fix for a minor (but very consistent)
    >> spelling error: 'desactivate' to 'deactivate'. This included
    tweaking an
    >> image with the word, changing some language files, and
    refactoring parts of
    >> the ManageButtonsPanel class.
    >
    > Committed all spelling corrections. Thanks.
    >
    >> 2. Does anyone know if there's a known issue with the chatalerter
    plugin?
    >> Eclipse has complained that the org.jdesktop.jdic.misc package
    can't be
    >> resolved ever since I checked it out via svn. However this has never
    >> prevented the SIP Communicator from running. I've tried asking a
    couple of
    >> times on the IRC channel but I think I keep catching everyone
    when they're
    >> asleep...
    >
    > Maybe you've already figured it out yourself, but I'm just
    replying for
    > the record..Even that Eclipse is complaining about a library, if you
    > compile and run SIP Communicator through Ant it will always work,
    > because the classpath is set inside the build.xml. If Eclipse is
    > complaining about a library this means that this library is not
    added in
    > the .classpath file of Eclipse. You need to do the following: right
    > click on the project / "Properties" / "Project Build Path" /
    "Libraries"
    > / "Add new" / Find the library in the lib directory of SComm.
    >
    >> 3. Couple of minor issues on the site:
    >> Developer Documentation > How to Configure Eclipse
    >> Minor simplification- it's not necessary to get the old version
    of Subclipse
    >> then upgrade it. The buckminster error can be avoided by
    disabling the
    >> optional components.
    >
    > Nice catch! I didn't experienced this error, so I'm not very familiar
    > with the problem, so again are you interested in changing the part of
    > the tutorial concerning the problem.
    >
    >> Developer Documentation > UI Service > Create and add
    configuration forms
    >> As Atul noticed the 'getConfigurationManager()' method doesn't
    exist and
    >> should be 'getConfigurationWindow()'.
    >
    > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
    this one,
    > could you be more precise?
    >
    > Damian, the patch was very well documented, very mature and easy to
    > read. Again you've done a very good work! Thank you for the
    patience and
    > keep up the good work:)
    >
    > Cheers,
    > Yana
    >
    >> Hope this helps. Cheers! -Galen
    >>
    >> PS. I'm planning to try going by my middle name, Galen, in SIP
    >> correspondence to avoid confusion with Damien and the other
    Damian. :slight_smile:
    >>
    ------------------------------------------------------------------------
    >>
    ------------------------------------------------------------------------
    >>
    ------------------------------------------------------------------------
    >>
    >> ---------------------------------------------------------------------
    >> To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

    >
    > ---------------------------------------------------------------------
    > To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

    >

    ---------------------------------------------------------------------
    To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hi. Thus far I've made the following changes:
Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

However, I've come up against a bit of a chicken-and-egg problem. The
swing-ui bundle's activator uses the keybindings service to initialize the
UI, hence that service needs to start first. However, the keybinding chooser
needs the swing-ui to set up the custom look-and-feel (the ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers! -Damian

···

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working (from
build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work, but
I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is that it
could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly
coupled with this service implementation and both are quite light then they
probably should belong to the same package.... It would have made sense to
keep them separated if there's a chance of someone needing concurrent
versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI and
having the code in there would make it easier... If you don't feel like
making the change then, by all means, just leave it to us. Your contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then
this makes sense, and the 'alternative bindings' are certainly substantial
enough. I'm sorry if I gave the impression that I wasn't willing to make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker soon
too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov <emcho@sip-communicator.org> >>> wrote:

Hey Damian,

Damian Johnson написа:

    First, I don't quite see the need of splitting the implementation

   into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the SIPCommFrame. If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should belong
to the same package.

    The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as a
plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

    It would also be nice if we could copy necessary classes from your

   lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor to
easily tweak the keybindings. If integrated we'd still probably need the
jar for that functionality (with the exception of the Persistence class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet (providing

an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

    Then, storing default and user preferences for key bindings in a

   binary form makes it difficult to change them for no real reason.
   I've just seen that your lib already allows to store settings in a
   properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite often
and not being able to do so by editing a text file would be a problem.

The

advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by manually
adding indexes. Here's a very rough example:

....
<impl>
  <keybindings>
      <binding1>
          <actionName value="Previous Tab"/>
          ...
      </binding1>
      <binding2>
          <actionName value="Next Tab"/>
          ...
      </binding2>
  </keybindings>
</impl>

    We can put the default ones into the defaults.properties files and

   then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look
into it.

Yes the defaults are new indeed.

    1. Would it be possible to implement support for alternative

   actions? Most key binding management UIs have it would be really
   nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,

   Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!

Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org >>>> >>> >>> <mailto:emcho@sip-communicator.org>> wrote:

   Hey Galen,

   Nice work indeed and the feature is quite useful. I've just tried it
and
   I like it a lot. Thanks for contributing!

   There are however a few things that make me feel uncomfortable with
the
   way this is currently integrated in SIP Communicator.

   First, I don't quite see the need of splitting the implementation
into a
   service and a plugin. The configuration UI could very well live in
   impl.keybinding. It would also be nice if we could copy necessary
   classes from your lib into the service implementation. There are two
   reasons I believe this would a good idea:
          - We'd probably need to make changes on some parts such as the
   configuration UI and the persistence mechanisms (see below) so that
they
   would fit the SC conventions.
          - It would make the code easier to understand.

   Then, storing default and user preferences for key bindings in a
binary
   form makes it difficult to change them for no real reason. I've just
   seen that your lib already allows to store settings in a properties
file
   so I think this would fit better. We can put the default ones into
the
   defaults.properties files and then use the configuration service to
   modify them.

   What do you think?

   Other than that I was wondering if I could make the following feature
   requests:

   1.Would it be possible to implement support for alternative actions?
   Most key binding management UIs have it would be really nice if we
   also did.
   and
   2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
   Ctrl-u shortcuts that we've recently added to the chat window?

   Once again, thanks for contributing your work!

   Emil

   Yana Stamcheva написа:
   > Hi Galen,
   >
   > you've done a really great job in this patch!
   >
   > First let me apologize for making you wait so long, we were really
   busy
   > lately. We have discussed the plugin off list already, but it's now
   > applied and committed. You're also on our contributors page.
   >
   > (more inline)
   >
   > Damian Johnson wrote:
   >> Hi. I've just finished integrating a chooser for keybindings into
   the SIP
   >> Communicator. This both provides a means for users to change their
   >> keybinding preferences and save them between runs. Most of the
   details of
   >> the addition are described in the latest blog post
   >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
   >> two additions:
   >> 1. Keybinding persistence via a new service. This saves custom
   bindings with
   >> the user's preferences and informs any frames using the bindings
   of updates.
   >> *src/net/java/sip/communicator/impl/keybindings
   >> src/net/java/sip/communicator/service/keybindings
   >
   > Applied. I like the way you have implemented key bindings in the
   gui bundle!
   >
   >> *
   >> 2. Plugin that provides a ConfigurationForm for changing the
   keybindings.
   >> *src/net/java/sip/communicator/plugin/keybindingChooser*
   >>
   >> I've added a how-to for adding new keybinding sets
   >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
the
   >> chooser description). Also, this is my first time making a patch
   >> - are binaries supposed to be provided separately like this? I
   didn't spot
   >> an option to include them in svn diff...
   >
   > My reply is coming late, but yes binaries should be supplied
   separately,
   > because they're not contained in the diff. The how-to is also very
   good
   > idea. Let me know if you're interested in adding this on the SIP
   > Communicator website.
   >
   >> A couple of usability features (any opinions?):
   >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
   >> 2. Keybindings aren't changed until the user hits "Apply".
   Perhaps bindings
   >> that haven't been applied should be discolored to draw attention
   to them?
   >
   > My personal opinion is that a button "Defaults" could be really
   helpful.
   > Otherwise coloring unsaved bindings could be useful, but also it
could
   > be confusing, the user could not be sure what exactly the color
means.
   >
   >> Known Issue:
   >> Not all permissible bindings work in all situations. For instance
   in the
   >> main frame the 'Contacts' and 'Chat rooms' panels appear to
   intercept some
   >> keyboard events, preventing bindings to directional arrows
(without
   >> modifiers) or the space bar from working. This isn't a terribly
   big woop but
   >> might possibly confuse users.
   >
   > Hm, could you explain more in details the problem situations, we
could
   > try to resolve this.
   >
   >> A few unrelated things I've come across:
   >> 1. The patch also includes the fix for a minor (but very
consistent)
   >> spelling error: 'desactivate' to 'deactivate'. This included
   tweaking an
   >> image with the word, changing some language files, and
   refactoring parts of
   >> the ManageButtonsPanel class.
   >
   > Committed all spelling corrections. Thanks.
   >
   >> 2. Does anyone know if there's a known issue with the chatalerter
   plugin?
   >> Eclipse has complained that the org.jdesktop.jdic.misc package
   can't be
   >> resolved ever since I checked it out via svn. However this has
never
   >> prevented the SIP Communicator from running. I've tried asking a
   couple of
   >> times on the IRC channel but I think I keep catching everyone
   when they're
   >> asleep...
   >
   > Maybe you've already figured it out yourself, but I'm just
   replying for
   > the record..Even that Eclipse is complaining about a library, if
you
   > compile and run SIP Communicator through Ant it will always work,
   > because the classpath is set inside the build.xml. If Eclipse is
   > complaining about a library this means that this library is not
   added in
   > the .classpath file of Eclipse. You need to do the following: right
   > click on the project / "Properties" / "Project Build Path" /
   "Libraries"
   > / "Add new" / Find the library in the lib directory of SComm.
   >
   >> 3. Couple of minor issues on the site:
   >> Developer Documentation > How to Configure Eclipse
   >> Minor simplification- it's not necessary to get the old version
   of Subclipse
   >> then upgrade it. The buckminster error can be avoided by
   disabling the
   >> optional components.
   >
   > Nice catch! I didn't experienced this error, so I'm not very
familiar
   > with the problem, so again are you interested in changing the part
of
   > the tutorial concerning the problem.
   >
   >> Developer Documentation > UI Service > Create and add
   configuration forms
   >> As Atul noticed the 'getConfigurationManager()' method doesn't
   exist and
   >> should be 'getConfigurationWindow()'.
   >
   > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
   this one,
   > could you be more precise?
   >
   > Damian, the patch was very well documented, very mature and easy to
   > read. Again you've done a very good work! Thank you for the
   patience and
   > keep up the good work:)
   >
   > Cheers,
   > Yana
   >
   >> Hope this helps. Cheers! -Galen
   >>
   >> PS. I'm planning to try going by my middle name, Galen, in SIP
   >> correspondence to avoid confusion with Damien and the other
   Damian. :slight_smile:
   >>
   >>
   >>
   >>

------------------------------------------------------------------------
   >>
   >>
   >>

------------------------------------------------------------------------
   >>
   >>
   >>

------------------------------------------------------------------------
   >>
   >>
---------------------------------------------------------------------
   >> To unsubscribe, e-mail:
   dev-unsubscribe@sip-communicator.dev.java.net
   <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:

   dev-help@sip-communicator.dev.java.net
   <mailto:dev-help@sip-communicator.dev.java.net>

    >

   >
---------------------------------------------------------------------
   > To unsubscribe, e-mail:
   dev-unsubscribe@sip-communicator.dev.java.net
   <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:

   dev-help@sip-communicator.dev.java.net
   <mailto:dev-help@sip-communicator.dev.java.net>

    >

   >

   ---------------------------------------------------------------------
   To unsubscribe, e-mail:
   dev-unsubscribe@sip-communicator.dev.java.net
   <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:

   dev-help@sip-communicator.dev.java.net
   <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

Hi Damian,

Damian Johnson wrote:

Hi. Thus far I've made the following changes:
Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

Good job!

However, I've come up against a bit of a chicken-and-egg problem. The
swing-ui bundle's activator uses the keybindings service to initialize the
UI, hence that service needs to start first. However, the keybinding chooser
needs the swing-ui to set up the custom look-and-feel (the ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers! -Damian

Sounds very good. I have also discussed this with Emil off list and he also agrees, so you could go ahead and integrate it in the swing-ui.

Just one more thing. I'm not sure if Emil has already asked, but is there any special reason for which the configuration files for keybindings : keybindings-chat and keybindings-main are binary files? Could we make them just properties files, so that they're easier to read?

Cheers,
Yana

···

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working (from
build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work, but
I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is that it
could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly
coupled with this service implementation and both are quite light then they
probably should belong to the same package.... It would have made sense to
keep them separated if there's a chance of someone needing concurrent
versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI and
having the code in there would make it easier... If you don't feel like
making the change then, by all means, just leave it to us. Your contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then
this makes sense, and the 'alternative bindings' are certainly substantial
enough. I'm sorry if I gave the impression that I wasn't willing to make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker soon
too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov <emcho@sip-communicator.org> >>>> wrote:

Hey Damian,

Damian Johnson ������:

    First, I don't quite see the need of splitting the implementation

   into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the SIPCommFrame. If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should belong
to the same package.

    The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as a
plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

    It would also be nice if we could copy necessary classes from your

   lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor to
easily tweak the keybindings. If integrated we'd still probably need the
jar for that functionality (with the exception of the Persistence class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet (providing

an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

    Then, storing default and user preferences for key bindings in a

   binary form makes it difficult to change them for no real reason.
   I've just seen that your lib already allows to store settings in a
   properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite often
and not being able to do so by editing a text file would be a problem.

The

advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by manually
adding indexes. Here's a very rough example:

....
<impl>
  <keybindings>
      <binding1>
          <actionName value="Previous Tab"/>
          ...
      </binding1>
      <binding2>
          <actionName value="Next Tab"/>
          ...
      </binding2>
  </keybindings>
</impl>

    We can put the default ones into the defaults.properties files and

   then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look
into it.

Yes the defaults are new indeed.

    1. Would it be possible to implement support for alternative

   actions? Most key binding management UIs have it would be really
   nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,

   Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!
Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org >>>>> >>>> <mailto:emcho@sip-communicator.org>> wrote:
   Hey Galen,

   Nice work indeed and the feature is quite useful. I've just tried it
and
   I like it a lot. Thanks for contributing!

   There are however a few things that make me feel uncomfortable with
the
   way this is currently integrated in SIP Communicator.

   First, I don't quite see the need of splitting the implementation
into a
   service and a plugin. The configuration UI could very well live in
   impl.keybinding. It would also be nice if we could copy necessary
   classes from your lib into the service implementation. There are two
   reasons I believe this would a good idea:
          - We'd probably need to make changes on some parts such as the
   configuration UI and the persistence mechanisms (see below) so that
they
   would fit the SC conventions.
          - It would make the code easier to understand.

   Then, storing default and user preferences for key bindings in a
binary
   form makes it difficult to change them for no real reason. I've just
   seen that your lib already allows to store settings in a properties
file
   so I think this would fit better. We can put the default ones into
the
   defaults.properties files and then use the configuration service to
   modify them.

   What do you think?

   Other than that I was wondering if I could make the following feature
   requests:

   1.Would it be possible to implement support for alternative actions?
   Most key binding management UIs have it would be really nice if we
   also did.
   and
   2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
   Ctrl-u shortcuts that we've recently added to the chat window?

   Once again, thanks for contributing your work!

   Emil

   Yana Stamcheva ������:
   > Hi Galen,
   >
   > you've done a really great job in this patch!
   >
   > First let me apologize for making you wait so long, we were really
   busy
   > lately. We have discussed the plugin off list already, but it's now
   > applied and committed. You're also on our contributors page.
   >
   > (more inline)
   >
   > Damian Johnson wrote:
   >> Hi. I've just finished integrating a chooser for keybindings into
   the SIP
   >> Communicator. This both provides a means for users to change their
   >> keybinding preferences and save them between runs. Most of the
   details of
   >> the addition are described in the latest blog post
   >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
   >> two additions:
   >> 1. Keybinding persistence via a new service. This saves custom
   bindings with
   >> the user's preferences and informs any frames using the bindings
   of updates.
   >> *src/net/java/sip/communicator/impl/keybindings
   >> src/net/java/sip/communicator/service/keybindings
   >
   > Applied. I like the way you have implemented key bindings in the
   gui bundle!
   >
   >> *
   >> 2. Plugin that provides a ConfigurationForm for changing the
   keybindings.
   >> *src/net/java/sip/communicator/plugin/keybindingChooser*
   >>
   >> I've added a how-to for adding new keybinding sets
   >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
the
   >> chooser description). Also, this is my first time making a patch
   >> - are binaries supposed to be provided separately like this? I
   didn't spot
   >> an option to include them in svn diff...
   >
   > My reply is coming late, but yes binaries should be supplied
   separately,
   > because they're not contained in the diff. The how-to is also very
   good
   > idea. Let me know if you're interested in adding this on the SIP
   > Communicator website.
   >
   >> A couple of usability features (any opinions?):
   >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
   >> 2. Keybindings aren't changed until the user hits "Apply".
   Perhaps bindings
   >> that haven't been applied should be discolored to draw attention
   to them?
   >
   > My personal opinion is that a button "Defaults" could be really
   helpful.
   > Otherwise coloring unsaved bindings could be useful, but also it
could
   > be confusing, the user could not be sure what exactly the color
means.
   >
   >> Known Issue:
   >> Not all permissible bindings work in all situations. For instance
   in the
   >> main frame the 'Contacts' and 'Chat rooms' panels appear to
   intercept some
   >> keyboard events, preventing bindings to directional arrows
(without
   >> modifiers) or the space bar from working. This isn't a terribly
   big woop but
   >> might possibly confuse users.
   >
   > Hm, could you explain more in details the problem situations, we
could
   > try to resolve this.
   >
   >> A few unrelated things I've come across:
   >> 1. The patch also includes the fix for a minor (but very
consistent)
   >> spelling error: 'desactivate' to 'deactivate'. This included
   tweaking an
   >> image with the word, changing some language files, and
   refactoring parts of
   >> the ManageButtonsPanel class.
   >
   > Committed all spelling corrections. Thanks.
   >
   >> 2. Does anyone know if there's a known issue with the chatalerter
   plugin?
   >> Eclipse has complained that the org.jdesktop.jdic.misc package
   can't be
   >> resolved ever since I checked it out via svn. However this has
never
   >> prevented the SIP Communicator from running. I've tried asking a
   couple of
   >> times on the IRC channel but I think I keep catching everyone
   when they're
   >> asleep...
   >
   > Maybe you've already figured it out yourself, but I'm just
   replying for
   > the record..Even that Eclipse is complaining about a library, if
you
   > compile and run SIP Communicator through Ant it will always work,
   > because the classpath is set inside the build.xml. If Eclipse is
   > complaining about a library this means that this library is not
   added in
   > the .classpath file of Eclipse. You need to do the following: right
   > click on the project / "Properties" / "Project Build Path" /
   "Libraries"
   > / "Add new" / Find the library in the lib directory of SComm.
   >
   >> 3. Couple of minor issues on the site:
   >> Developer Documentation > How to Configure Eclipse
   >> Minor simplification- it's not necessary to get the old version
   of Subclipse
   >> then upgrade it. The buckminster error can be avoided by
   disabling the
   >> optional components.
   >
   > Nice catch! I didn't experienced this error, so I'm not very
familiar
   > with the problem, so again are you interested in changing the part
of
   > the tutorial concerning the problem.
   >
   >> Developer Documentation > UI Service > Create and add
   configuration forms
   >> As Atul noticed the 'getConfigurationManager()' method doesn't
   exist and
   >> should be 'getConfigurationWindow()'.
   >
   > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
   this one,
   > could you be more precise?
   >
   > Damian, the patch was very well documented, very mature and easy to
   > read. Again you've done a very good work! Thank you for the
   patience and
   > keep up the good work:)
   >
   > Cheers,
   > Yana
   >
   >> Hope this helps. Cheers! -Galen
   >>
   >> PS. I'm planning to try going by my middle name, Galen, in SIP
   >> correspondence to avoid confusion with Damien and the other
   Damian. :slight_smile:
   >>

------------------------------------------------------------------------
   >>

------------------------------------------------------------------------
   >>

------------------------------------------------------------------------
   >>
---------------------------------------------------------------------
   >> To unsubscribe, e-mail:
   dev-unsubscribe@sip-communicator.dev.java.net
   <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:

   dev-help@sip-communicator.dev.java.net
   <mailto:dev-help@sip-communicator.dev.java.net>

    >

   >
---------------------------------------------------------------------
   > To unsubscribe, e-mail:
   dev-unsubscribe@sip-communicator.dev.java.net
   <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:

   dev-help@sip-communicator.dev.java.net
   <mailto:dev-help@sip-communicator.dev.java.net>

    >

   >

   ---------------------------------------------------------------------
   To unsubscribe, e-mail:
   dev-unsubscribe@sip-communicator.dev.java.net
   <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:

   dev-help@sip-communicator.dev.java.net
   <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Hi Yana. Thanks for the quick response! Previous discussion on this thread
concerned the binary property files. I chose serialized linked hash maps so
ordering would be preserved (which is important for the appearance). Emil's
suggested adding an index to the plain text properties and I'm planning to
look into this soon. -Damian

···

2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:

Hi Damian,

Damian Johnson wrote:

Hi. Thus far I've made the following changes:
Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

Good job!

However, I've come up against a bit of a chicken-and-egg problem. The
swing-ui bundle's activator uses the keybindings service to initialize the
UI, hence that service needs to start first. However, the keybinding
chooser
needs the swing-ui to set up the custom look-and-feel (the
ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers!
-Damian

Sounds very good. I have also discussed this with Emil off list and he also
agrees, so you could go ahead and integrate it in the swing-ui.

Just one more thing. I'm not sure if Emil has already asked, but is there
any special reason for which the configuration files for keybindings :
keybindings-chat and keybindings-main are binary files? Could we make them
just properties files, so that they're easier to read?

Cheers,
Yana

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working (from
build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work,
but
I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is that it
could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of
the
plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the
proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly
coupled with this service implementation and both are quite light then
they
probably should belong to the same package.... It would have made sense
to
keep them separated if there's a chance of someone needing concurrent
versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the UI
and
having the code in there would make it easier... If you don't feel like
making the change then, by all means, just leave it to us. Your
contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then
this makes sense, and the 'alternative bindings' are certainly
substantial
enough. I'm sorry if I gave the impression that I wasn't willing to
make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker
soon
too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < >>>>> emcho@sip-communicator.org> >>>>> wrote:

Hey Damian,

Damian Johnson написа:

   First, I don't quite see the need of splitting the implementation

  into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins
to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the SIPCommFrame.
If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of

the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should
belong
to the same package.

   The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as
a
plugin.

I agree, developers are likely to tackle the UI but since both the

plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

   It would also be nice if we could copy necessary classes from your

  lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this
is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor
to
easily tweak the keybindings. If integrated we'd still probably need
the
jar for that functionality (with the exception of the Persistence
class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text

form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet
(providing

an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give
a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in

the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

   Then, storing default and user preferences for key bindings in a

  binary form makes it difficult to change them for no real reason.
  I've just seen that your lib already allows to store settings in a
  properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite

often
and not being able to do so by editing a text file would be a problem.

The

advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by

manually
adding indexes. Here's a very rough example:

....
<impl>
<keybindings>
     <binding1>
         <actionName value="Previous Tab"/>
         ...
     </binding1>
     <binding2>
         <actionName value="Next Tab"/>
         ...
     </binding2>
</keybindings>
</impl>

   We can put the default ones into the defaults.properties files and

  then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look
into it.

Yes the defaults are new indeed.

   1. Would it be possible to implement support for alternative

  actions? Most key binding management UIs have it would be really
  nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

   2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,

  Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple
once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!
Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < >>>>>> emcho@sip-communicator.org >>>>>> >>>>>> <mailto:emcho@sip-communicator.org>> wrote:

  Hey Galen,

  Nice work indeed and the feature is quite useful. I've just tried it
and
  I like it a lot. Thanks for contributing!

  There are however a few things that make me feel uncomfortable with
the
  way this is currently integrated in SIP Communicator.

  First, I don't quite see the need of splitting the implementation
into a
  service and a plugin. The configuration UI could very well live in
  impl.keybinding. It would also be nice if we could copy necessary
  classes from your lib into the service implementation. There are two
  reasons I believe this would a good idea:
         - We'd probably need to make changes on some parts such as
the
  configuration UI and the persistence mechanisms (see below) so that
they
  would fit the SC conventions.
         - It would make the code easier to understand.

  Then, storing default and user preferences for key bindings in a
binary
  form makes it difficult to change them for no real reason. I've just
  seen that your lib already allows to store settings in a properties
file
  so I think this would fit better. We can put the default ones into
the
  defaults.properties files and then use the configuration service to
  modify them.

  What do you think?

  Other than that I was wondering if I could make the following
feature
  requests:

  1.Would it be possible to implement support for alternative actions?
  Most key binding management UIs have it would be really nice if we
  also did.
  and
  2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
  Ctrl-u shortcuts that we've recently added to the chat window?

  Once again, thanks for contributing your work!

  Emil

  Yana Stamcheva написа:
  > Hi Galen,
  >
  > you've done a really great job in this patch!
  >
  > First let me apologize for making you wait so long, we were really
  busy
  > lately. We have discussed the plugin off list already, but it's
now
  > applied and committed. You're also on our contributors page.
  >
  > (more inline)
  >
  > Damian Johnson wrote:
  >> Hi. I've just finished integrating a chooser for keybindings into
  the SIP
  >> Communicator. This both provides a means for users to change
their
  >> keybinding preferences and save them between runs. Most of the
  details of
  >> the addition are described in the latest blog post
  >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
  >> two additions:
  >> 1. Keybinding persistence via a new service. This saves custom
  bindings with
  >> the user's preferences and informs any frames using the bindings
  of updates.
  >> *src/net/java/sip/communicator/impl/keybindings
  >> src/net/java/sip/communicator/service/keybindings
  >
  > Applied. I like the way you have implemented key bindings in the
  gui bundle!
  >
  >> *
  >> 2. Plugin that provides a ConfigurationForm for changing the
  keybindings.
  >> *src/net/java/sip/communicator/plugin/keybindingChooser*
  >>
  >> I've added a how-to for adding new keybinding sets
  >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
the
  >> chooser description). Also, this is my first time making a patch
  >> - are binaries supposed to be provided separately like this? I
  didn't spot
  >> an option to include them in svn diff...
  >
  > My reply is coming late, but yes binaries should be supplied
  separately,
  > because they're not contained in the diff. The how-to is also very
  good
  > idea. Let me know if you're interested in adding this on the SIP
  > Communicator website.
  >
  >> A couple of usability features (any opinions?):
  >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
  >> 2. Keybindings aren't changed until the user hits "Apply".
  Perhaps bindings
  >> that haven't been applied should be discolored to draw attention
  to them?
  >
  > My personal opinion is that a button "Defaults" could be really
  helpful.
  > Otherwise coloring unsaved bindings could be useful, but also it
could
  > be confusing, the user could not be sure what exactly the color
means.
  >
  >> Known Issue:
  >> Not all permissible bindings work in all situations. For instance
  in the
  >> main frame the 'Contacts' and 'Chat rooms' panels appear to
  intercept some
  >> keyboard events, preventing bindings to directional arrows
(without
  >> modifiers) or the space bar from working. This isn't a terribly
  big woop but
  >> might possibly confuse users.
  >
  > Hm, could you explain more in details the problem situations, we
could
  > try to resolve this.
  >
  >> A few unrelated things I've come across:
  >> 1. The patch also includes the fix for a minor (but very
consistent)
  >> spelling error: 'desactivate' to 'deactivate'. This included
  tweaking an
  >> image with the word, changing some language files, and
  refactoring parts of
  >> the ManageButtonsPanel class.
  >
  > Committed all spelling corrections. Thanks.
  >
  >> 2. Does anyone know if there's a known issue with the chatalerter
  plugin?
  >> Eclipse has complained that the org.jdesktop.jdic.misc package
  can't be
  >> resolved ever since I checked it out via svn. However this has
never
  >> prevented the SIP Communicator from running. I've tried asking a
  couple of
  >> times on the IRC channel but I think I keep catching everyone
  when they're
  >> asleep...
  >
  > Maybe you've already figured it out yourself, but I'm just
  replying for
  > the record..Even that Eclipse is complaining about a library, if
you
  > compile and run SIP Communicator through Ant it will always work,
  > because the classpath is set inside the build.xml. If Eclipse is
  > complaining about a library this means that this library is not
  added in
  > the .classpath file of Eclipse. You need to do the following:
right
  > click on the project / "Properties" / "Project Build Path" /
  "Libraries"
  > / "Add new" / Find the library in the lib directory of SComm.
  >
  >> 3. Couple of minor issues on the site:
  >> Developer Documentation > How to Configure Eclipse
  >> Minor simplification- it's not necessary to get the old version
  of Subclipse
  >> then upgrade it. The buckminster error can be avoided by
  disabling the
  >> optional components.
  >
  > Nice catch! I didn't experienced this error, so I'm not very
familiar
  > with the problem, so again are you interested in changing the part
of
  > the tutorial concerning the problem.
  >
  >> Developer Documentation > UI Service > Create and add
  configuration forms
  >> As Atul noticed the 'getConfigurationManager()' method doesn't
  exist and
  >> should be 'getConfigurationWindow()'.
  >
  > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
  this one,
  > could you be more precise?
  >
  > Damian, the patch was very well documented, very mature and easy
to
  > read. Again you've done a very good work! Thank you for the
  patience and
  > keep up the good work:)
  >
  > Cheers,
  > Yana
  >
  >> Hope this helps. Cheers! -Galen
  >>
  >> PS. I'm planning to try going by my middle name, Galen, in SIP
  >> correspondence to avoid confusion with Damien and the other
  Damian. :slight_smile:
  >>
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
---------------------------------------------------------------------
  >> To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >

  >
---------------------------------------------------------------------
  > To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >

  >

---------------------------------------------------------------------
  To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#7

Hi. I've tweaked the KeybindingsService implementation to utilize the
ConfigurationService so the custom bindings are now stored in a
human-editable fashion. However, I'm having some difficulty finding where
the defaults should be stored. The obvious choice is
'resources/config/defaults.properties' but the ResourceManagementService,
unlike ConfigurationService, isn't structured as a tree and hence doesn't
have any methods for retrieving batches of defaults (ie, all chat
keybindings). Per chance is there a file with the initial values of
'~/.sip-communicator/sip-communicator.xml'? -Damian

···

2008/7/25 Damian Johnson <atagar1@gmail.com>

Hi Yana. Thanks for the quick response! Previous discussion on this thread
concerned the binary property files. I chose serialized linked hash maps so
ordering would be preserved (which is important for the appearance). Emil's
suggested adding an index to the plain text properties and I'm planning to
look into this soon. -Damian

2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:

Hi Damian,

Damian Johnson wrote:

Hi. Thus far I've made the following changes:
Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching
that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

Good job!

However, I've come up against a bit of a chicken-and-egg problem. The
swing-ui bundle's activator uses the keybindings service to initialize
the
UI, hence that service needs to start first. However, the keybinding
chooser
needs the swing-ui to set up the custom look-and-feel (the
ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers!
-Damian

Sounds very good. I have also discussed this with Emil off list and he
also agrees, so you could go ahead and integrate it in the swing-ui.

Just one more thing. I'm not sure if Emil has already asked, but is there
any special reason for which the configuration files for keybindings :
keybindings-chat and keybindings-main are binary files? Could we make them
just properties files, so that they're easier to read?

Cheers,
Yana

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working (from
build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work,
but
I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is that
it
could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of
the
plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the
proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly
coupled with this service implementation and both are quite light then
they
probably should belong to the same package.... It would have made
sense to
keep them separated if there's a chance of someone needing concurrent
versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the UI
and
having the code in there would make it easier... If you don't feel
like
making the change then, by all means, just leave it to us. Your
contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then
this makes sense, and the 'alternative bindings' are certainly
substantial
enough. I'm sorry if I gave the impression that I wasn't willing to
make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker
soon
too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < >>>>>> emcho@sip-communicator.org> >>>>>> wrote:

Hey Damian,

Damian Johnson написа:

   First, I don't quite see the need of splitting the implementation

  into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins
to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the SIPCommFrame.
If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of

the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should
belong
to the same package.

   The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as
a
plugin.

I agree, developers are likely to tackle the UI but since both the

plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code
would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

   It would also be nice if we could copy necessary classes from your

  lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so
getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this
is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor
to
easily tweak the keybindings. If integrated we'd still probably need
the
jar for that functionality (with the exception of the Persistence
class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text

form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet
(providing

an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give
a
lot more code to look at.

Well unless I am missing something we'll probably only need what's

in
the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just
leave
it to us. Your contribution is appreciated as it is.

   Then, storing default and user preferences for key bindings in a

  binary form makes it difficult to change them for no real reason.
  I've just seen that your lib already allows to store settings in a
  properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite

often
and not being able to do so by editing a text file would be a problem.

The

advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI
perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by

manually
adding indexes. Here's a very rough example:

....
<impl>
<keybindings>
     <binding1>
         <actionName value="Previous Tab"/>
         ...
     </binding1>
     <binding2>
         <actionName value="Next Tab"/>
         ...
     </binding2>
</keybindings>
</impl>

   We can put the default ones into the defaults.properties files and

  then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll
look
into it.

Yes the defaults are new indeed.

   1. Would it be possible to implement support for alternative

  actions? Most key binding management UIs have it would be really
  nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

   2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,

  Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple
once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!
Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < >>>>>>> emcho@sip-communicator.org >>>>>>> >>>>>>> <mailto:emcho@sip-communicator.org>> wrote:

  Hey Galen,

  Nice work indeed and the feature is quite useful. I've just tried
it
and
  I like it a lot. Thanks for contributing!

  There are however a few things that make me feel uncomfortable with
the
  way this is currently integrated in SIP Communicator.

  First, I don't quite see the need of splitting the implementation
into a
  service and a plugin. The configuration UI could very well live in
  impl.keybinding. It would also be nice if we could copy necessary
  classes from your lib into the service implementation. There are
two
  reasons I believe this would a good idea:
         - We'd probably need to make changes on some parts such as
the
  configuration UI and the persistence mechanisms (see below) so that
they
  would fit the SC conventions.
         - It would make the code easier to understand.

  Then, storing default and user preferences for key bindings in a
binary
  form makes it difficult to change them for no real reason. I've
just
  seen that your lib already allows to store settings in a properties
file
  so I think this would fit better. We can put the default ones into
the
  defaults.properties files and then use the configuration service to
  modify them.

  What do you think?

  Other than that I was wondering if I could make the following
feature
  requests:

  1.Would it be possible to implement support for alternative
actions?
  Most key binding management UIs have it would be really nice if we
  also did.
  and
  2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,
  Ctrl-u shortcuts that we've recently added to the chat window?

  Once again, thanks for contributing your work!

  Emil

  Yana Stamcheva написа:
  > Hi Galen,
  >
  > you've done a really great job in this patch!
  >
  > First let me apologize for making you wait so long, we were
really
  busy
  > lately. We have discussed the plugin off list already, but it's
now
  > applied and committed. You're also on our contributors page.
  >
  > (more inline)
  >
  > Damian Johnson wrote:
  >> Hi. I've just finished integrating a chooser for keybindings
into
  the SIP
  >> Communicator. This both provides a means for users to change
their
  >> keybinding preferences and save them between runs. Most of the
  details of
  >> the addition are described in the latest blog post
  >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
  >> two additions:
  >> 1. Keybinding persistence via a new service. This saves custom
  bindings with
  >> the user's preferences and informs any frames using the bindings
  of updates.
  >> *src/net/java/sip/communicator/impl/keybindings
  >> src/net/java/sip/communicator/service/keybindings
  >
  > Applied. I like the way you have implemented key bindings in the
  gui bundle!
  >
  >> *
  >> 2. Plugin that provides a ConfigurationForm for changing the
  keybindings.
  >> *src/net/java/sip/communicator/plugin/keybindingChooser*
  >>
  >> I've added a how-to for adding new keybinding sets
  >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
the
  >> chooser description). Also, this is my first time making a patch
  >> - are binaries supposed to be provided separately like this? I
  didn't spot
  >> an option to include them in svn diff...
  >
  > My reply is coming late, but yes binaries should be supplied
  separately,
  > because they're not contained in the diff. The how-to is also
very
  good
  > idea. Let me know if you're interested in adding this on the SIP
  > Communicator website.
  >
  >> A couple of usability features (any opinions?):
  >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
  >> 2. Keybindings aren't changed until the user hits "Apply".
  Perhaps bindings
  >> that haven't been applied should be discolored to draw attention
  to them?
  >
  > My personal opinion is that a button "Defaults" could be really
  helpful.
  > Otherwise coloring unsaved bindings could be useful, but also it
could
  > be confusing, the user could not be sure what exactly the color
means.
  >
  >> Known Issue:
  >> Not all permissible bindings work in all situations. For
instance
  in the
  >> main frame the 'Contacts' and 'Chat rooms' panels appear to
  intercept some
  >> keyboard events, preventing bindings to directional arrows
(without
  >> modifiers) or the space bar from working. This isn't a terribly
  big woop but
  >> might possibly confuse users.
  >
  > Hm, could you explain more in details the problem situations, we
could
  > try to resolve this.
  >
  >> A few unrelated things I've come across:
  >> 1. The patch also includes the fix for a minor (but very
consistent)
  >> spelling error: 'desactivate' to 'deactivate'. This included
  tweaking an
  >> image with the word, changing some language files, and
  refactoring parts of
  >> the ManageButtonsPanel class.
  >
  > Committed all spelling corrections. Thanks.
  >
  >> 2. Does anyone know if there's a known issue with the
chatalerter
  plugin?
  >> Eclipse has complained that the org.jdesktop.jdic.misc package
  can't be
  >> resolved ever since I checked it out via svn. However this has
never
  >> prevented the SIP Communicator from running. I've tried asking a
  couple of
  >> times on the IRC channel but I think I keep catching everyone
  when they're
  >> asleep...
  >
  > Maybe you've already figured it out yourself, but I'm just
  replying for
  > the record..Even that Eclipse is complaining about a library, if
you
  > compile and run SIP Communicator through Ant it will always work,
  > because the classpath is set inside the build.xml. If Eclipse is
  > complaining about a library this means that this library is not
  added in
  > the .classpath file of Eclipse. You need to do the following:
right
  > click on the project / "Properties" / "Project Build Path" /
  "Libraries"
  > / "Add new" / Find the library in the lib directory of SComm.
  >
  >> 3. Couple of minor issues on the site:
  >> Developer Documentation > How to Configure Eclipse
  >> Minor simplification- it's not necessary to get the old version
  of Subclipse
  >> then upgrade it. The buckminster error can be avoided by
  disabling the
  >> optional components.
  >
  > Nice catch! I didn't experienced this error, so I'm not very
familiar
  > with the problem, so again are you interested in changing the
part
of
  > the tutorial concerning the problem.
  >
  >> Developer Documentation > UI Service > Create and add
  configuration forms
  >> As Atul noticed the 'getConfigurationManager()' method doesn't
  exist and
  >> should be 'getConfigurationWindow()'.
  >
  > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
  this one,
  > could you be more precise?
  >
  > Damian, the patch was very well documented, very mature and easy
to
  > read. Again you've done a very good work! Thank you for the
  patience and
  > keep up the good work:)
  >
  > Cheers,
  > Yana
  >
  >> Hope this helps. Cheers! -Galen
  >>
  >> PS. I'm planning to try going by my middle name, Galen, in SIP
  >> correspondence to avoid confusion with Damien and the other
  Damian. :slight_smile:
  >>
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
---------------------------------------------------------------------
  >> To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >

  >
---------------------------------------------------------------------
  > To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >

  >

---------------------------------------------------------------------
  To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#8

Hello Damian,

I've work on the configuration service (for my GSoC project) and every
data is created by the classes which want to store data. If the file
doesn't exists, the ConfigurationServiceImpl just call the method:
"createPropertiesDocument" which just create the root of the tree.

The best way, is to store the default data like you said in the
resources directory and when you bundle is running, checks if the data
are stored in the configuration file. If it's not the case, use a
method to add them by reading the key's in the default data file.

Bye

Damien

···

2008/7/27 Damian Johnson <atagar1@gmail.com>:

Hi. I've tweaked the KeybindingsService implementation to utilize the
ConfigurationService so the custom bindings are now stored in a
human-editable fashion. However, I'm having some difficulty finding where
the defaults should be stored. The obvious choice is
'resources/config/defaults.properties' but the ResourceManagementService,
unlike ConfigurationService, isn't structured as a tree and hence doesn't
have any methods for retrieving batches of defaults (ie, all chat
keybindings). Per chance is there a file with the initial values of
'~/.sip-communicator/sip-communicator.xml'? -Damian

2008/7/25 Damian Johnson <atagar1@gmail.com>

Hi Yana. Thanks for the quick response! Previous discussion on this thread
concerned the binary property files. I chose serialized linked hash maps so
ordering would be preserved (which is important for the appearance). Emil's
suggested adding an index to the plain text properties and I'm planning to
look into this soon. -Damian

2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:

Hi Damian,

Damian Johnson wrote:

Hi. Thus far I've made the following changes:
Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching
that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

Good job!

However, I've come up against a bit of a chicken-and-egg problem. The
swing-ui bundle's activator uses the keybindings service to initialize
the
UI, hence that service needs to start first. However, the keybinding
chooser
needs the swing-ui to set up the custom look-and-feel (the
ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers!
-Damian

Sounds very good. I have also discussed this with Emil off list and he
also agrees, so you could go ahead and integrate it in the swing-ui.

Just one more thing. I'm not sure if Emil has already asked, but is there
any special reason for which the configuration files for keybindings :
keybindings-chat and keybindings-main are binary files? Could we make them
just properties files, so that they're easier to read?

Cheers,
Yana

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working (from
build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work,
but
I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is that
it
could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now
don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of
the
plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the
proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly
coupled with this service implementation and both are quite light
then they
probably should belong to the same package.... It would have made
sense to
keep them separated if there's a chance of someone needing concurrent
versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's
in
the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the
UI and
having the code in there would make it easier... If you don't feel
like
making the change then, by all means, just leave it to us. Your
contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then
this makes sense, and the 'alternative bindings' are certainly
substantial
enough. I'm sorry if I gave the impression that I wasn't willing to
make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker
soon
too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in
my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov >>>>>>> <emcho@sip-communicator.org> >>>>>>> wrote:

Hey Damian,

Damian Johnson написа:

   First, I don't quite see the need of splitting the implementation

  into a service and a plugin.

Actually I was just thinking about this. Are we going to want
plugins to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the
SIPCommFrame. If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of
the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should
belong
to the same package.

   The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act
as a
plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code
would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance
of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

   It would also be nice if we could copy necessary classes from your

  lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so
getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this
is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor
to
easily tweak the keybindings. If integrated we'd still probably need
the
jar for that functionality (with the exception of the Persistence
class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text
form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet
(providing

an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly
give a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the
UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just
leave
it to us. Your contribution is appreciated as it is.

   Then, storing default and user preferences for key bindings in a

  binary form makes it difficult to change them for no real reason.
  I've just seen that your lib already allows to store settings in a
  properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite
often
and not being able to do so by editing a text file would be a
problem.

The

advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI
perspective
(so options like 'next tab' and 'previous tab' are shown next to
each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by
manually
adding indexes. Here's a very rough example:

....
<impl>
<keybindings>
     <binding1>
         <actionName value="Previous Tab"/>
         ...
     </binding1>
     <binding2>
         <actionName value="Next Tab"/>
         ...
     </binding2>
</keybindings>
</impl>

   We can put the default ones into the defaults.properties files and

  then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll
look
into it.

Yes the defaults are new indeed.

   1. Would it be possible to implement support for alternative

  actions? Most key binding management UIs have it would be really
  nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

   2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,

  Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple
once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!
Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov >>>>>>>> <emcho@sip-communicator.org >>>>>>>> >>>>>>> <mailto:emcho@sip-communicator.org>> wrote:

  Hey Galen,

  Nice work indeed and the feature is quite useful. I've just tried
it
and
  I like it a lot. Thanks for contributing!

  There are however a few things that make me feel uncomfortable
with
the
  way this is currently integrated in SIP Communicator.

  First, I don't quite see the need of splitting the implementation
into a
  service and a plugin. The configuration UI could very well live in
  impl.keybinding. It would also be nice if we could copy necessary
  classes from your lib into the service implementation. There are
two
  reasons I believe this would a good idea:
         - We'd probably need to make changes on some parts such as
the
  configuration UI and the persistence mechanisms (see below) so
that
they
  would fit the SC conventions.
         - It would make the code easier to understand.

  Then, storing default and user preferences for key bindings in a
binary
  form makes it difficult to change them for no real reason. I've
just
  seen that your lib already allows to store settings in a
properties
file
  so I think this would fit better. We can put the default ones into
the
  defaults.properties files and then use the configuration service
to
  modify them.

  What do you think?

  Other than that I was wondering if I could make the following
feature
  requests:

  1.Would it be possible to implement support for alternative
actions?
  Most key binding management UIs have it would be really nice if we
  also did.
  and
  2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,
  Ctrl-u shortcuts that we've recently added to the chat window?

  Once again, thanks for contributing your work!

  Emil

  Yana Stamcheva написа:
  > Hi Galen,
  >
  > you've done a really great job in this patch!
  >
  > First let me apologize for making you wait so long, we were
really
  busy
  > lately. We have discussed the plugin off list already, but it's
now
  > applied and committed. You're also on our contributors page.
  >
  > (more inline)
  >
  > Damian Johnson wrote:
  >> Hi. I've just finished integrating a chooser for keybindings
into
  the SIP
  >> Communicator. This both provides a means for users to change
their
  >> keybinding preferences and save them between runs. Most of the
  details of
  >> the addition are described in the latest blog post
  >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
  >> two additions:
  >> 1. Keybinding persistence via a new service. This saves custom
  bindings with
  >> the user's preferences and informs any frames using the
bindings
  of updates.
  >> *src/net/java/sip/communicator/impl/keybindings
  >> src/net/java/sip/communicator/service/keybindings
  >
  > Applied. I like the way you have implemented key bindings in the
  gui bundle!
  >
  >> *
  >> 2. Plugin that provides a ConfigurationForm for changing the
  keybindings.
  >> *src/net/java/sip/communicator/plugin/keybindingChooser*
  >>
  >> I've added a how-to for adding new keybinding sets
  >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
the
  >> chooser description). Also, this is my first time making a
patch
  >> - are binaries supposed to be provided separately like this? I
  didn't spot
  >> an option to include them in svn diff...
  >
  > My reply is coming late, but yes binaries should be supplied
  separately,
  > because they're not contained in the diff. The how-to is also
very
  good
  > idea. Let me know if you're interested in adding this on the SIP
  > Communicator website.
  >
  >> A couple of usability features (any opinions?):
  >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
  >> 2. Keybindings aren't changed until the user hits "Apply".
  Perhaps bindings
  >> that haven't been applied should be discolored to draw
attention
  to them?
  >
  > My personal opinion is that a button "Defaults" could be really
  helpful.
  > Otherwise coloring unsaved bindings could be useful, but also it
could
  > be confusing, the user could not be sure what exactly the color
means.
  >
  >> Known Issue:
  >> Not all permissible bindings work in all situations. For
instance
  in the
  >> main frame the 'Contacts' and 'Chat rooms' panels appear to
  intercept some
  >> keyboard events, preventing bindings to directional arrows
(without
  >> modifiers) or the space bar from working. This isn't a terribly
  big woop but
  >> might possibly confuse users.
  >
  > Hm, could you explain more in details the problem situations, we
could
  > try to resolve this.
  >
  >> A few unrelated things I've come across:
  >> 1. The patch also includes the fix for a minor (but very
consistent)
  >> spelling error: 'desactivate' to 'deactivate'. This included
  tweaking an
  >> image with the word, changing some language files, and
  refactoring parts of
  >> the ManageButtonsPanel class.
  >
  > Committed all spelling corrections. Thanks.
  >
  >> 2. Does anyone know if there's a known issue with the
chatalerter
  plugin?
  >> Eclipse has complained that the org.jdesktop.jdic.misc package
  can't be
  >> resolved ever since I checked it out via svn. However this has
never
  >> prevented the SIP Communicator from running. I've tried asking
a
  couple of
  >> times on the IRC channel but I think I keep catching everyone
  when they're
  >> asleep...
  >
  > Maybe you've already figured it out yourself, but I'm just
  replying for
  > the record..Even that Eclipse is complaining about a library, if
you
  > compile and run SIP Communicator through Ant it will always
work,
  > because the classpath is set inside the build.xml. If Eclipse is
  > complaining about a library this means that this library is not
  added in
  > the .classpath file of Eclipse. You need to do the following:
right
  > click on the project / "Properties" / "Project Build Path" /
  "Libraries"
  > / "Add new" / Find the library in the lib directory of SComm.
  >
  >> 3. Couple of minor issues on the site:
  >> Developer Documentation > How to Configure Eclipse
  >> Minor simplification- it's not necessary to get the old version
  of Subclipse
  >> then upgrade it. The buckminster error can be avoided by
  disabling the
  >> optional components.
  >
  > Nice catch! I didn't experienced this error, so I'm not very
familiar
  > with the problem, so again are you interested in changing the
part
of
  > the tutorial concerning the problem.
  >
  >> Developer Documentation > UI Service > Create and add
  configuration forms
  >> As Atul noticed the 'getConfigurationManager()' method doesn't
  exist and
  >> should be 'getConfigurationWindow()'.
  >
  > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
  this one,
  > could you be more precise?
  >
  > Damian, the patch was very well documented, very mature and easy
to
  > read. Again you've done a very good work! Thank you for the
  patience and
  > keep up the good work:)
  >
  > Cheers,
  > Yana
  >
  >> Hope this helps. Cheers! -Galen
  >>
  >> PS. I'm planning to try going by my middle name, Galen, in SIP
  >> correspondence to avoid confusion with Damien and the other
  Damian. :slight_smile:
  >>
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>

---------------------------------------------------------------------
  >> To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

   >> For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

   >

  >

---------------------------------------------------------------------
  > To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

   > For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

   >

  >

---------------------------------------------------------------------
  To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

   For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

--
Damien ROTH
Programmeur n.m : Celui qui résout un problème que vous n'aviez pas,
d'une façon que vous ne comprenez pas.


#9

Hey Damian,

Glad to hear you are making progress on this one!

The defaults.properties file was indeed previewed for cases just like
yours: the conf service is supposed to load it as default content the
first time it runs.

However, as you have noticed this behaviour hasn't been fully
implemented yet so right now you can either implement it yourself or
do as Damien suggested - use a resource file inside your bundle and in
case your properties are not already in the ConfigurationService, load
them in there by your self.

Cheers
Emil

···

On 7/27/08, Damian Johnson <atagar1@gmail.com> wrote:

Hi. I've tweaked the KeybindingsService implementation to utilize the
ConfigurationService so the custom bindings are now stored in a
human-editable fashion. However, I'm having some difficulty finding where
the defaults should be stored. The obvious choice is
'resources/config/defaults.properties' but the ResourceManagementService,
unlike ConfigurationService, isn't structured as a tree and hence doesn't
have any methods for retrieving batches of defaults (ie, all chat
keybindings). Per chance is there a file with the initial values of
'~/.sip-communicator/sip-communicator.xml'? -Damian

2008/7/25 Damian Johnson <atagar1@gmail.com>

Hi Yana. Thanks for the quick response! Previous discussion on this thread
concerned the binary property files. I chose serialized linked hash maps
so
ordering would be preserved (which is important for the appearance).
Emil's
suggested adding an index to the plain text properties and I'm planning to
look into this soon. -Damian

2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:

Hi Damian,

Damian Johnson wrote:

Hi. Thus far I've made the following changes:
Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching
that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

Good job!

However, I've come up against a bit of a chicken-and-egg problem. The
swing-ui bundle's activator uses the keybindings service to initialize
the
UI, hence that service needs to start first. However, the keybinding
chooser
needs the swing-ui to set up the custom look-and-feel (the
ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers!
-Damian

Sounds very good. I have also discussed this with Emil off list and he
also agrees, so you could go ahead and integrate it in the swing-ui.

Just one more thing. I'm not sure if Emil has already asked, but is there
any special reason for which the configuration files for keybindings :
keybindings-chat and keybindings-main are binary files? Could we make
them
just properties files, so that they're easier to read?

Cheers,
Yana

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working (from
build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's work,
but
I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is that
it
could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now
don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content of
the
plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the
proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite tightly
coupled with this service implementation and both are quite light
then
they
probably should belong to the same package.... It would have made
sense to
keep them separated if there's a chance of someone needing concurrent
versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's
in
the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the
UI
and
having the code in there would make it easier... If you don't feel
like
making the change then, by all means, just leave it to us. Your
contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI then
this makes sense, and the 'alternative bindings' are certainly
substantial
enough. I'm sorry if I gave the impression that I wasn't willing to
make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell checker
soon
too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in
my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < >>>>>>> emcho@sip-communicator.org> >>>>>>> wrote:

Hey Damian,

Damian Johnson написа:

   First, I don't quite see the need of splitting the implementation

  into a service and a plugin.

Actually I was just thinking about this. Are we going to want
plugins
to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the
SIPCommFrame.
If
not then you're completely right.

I am not sure I see why this prevents us from copying the content
of

the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should
belong
to the same package.

   The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act
as
a
plugin.

I agree, developers are likely to tackle the UI but since both the

plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code
would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance
of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

   It would also be nice if we could copy necessary classes from your

  lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so
getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this
is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor
to
easily tweak the keybindings. If integrated we'd still probably need
the
jar for that functionality (with the exception of the Persistence
class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text

form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet
(providing

an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly
give
a
lot more code to look at.

Well unless I am missing something we'll probably only need what's

in
the chooser package which gives us approximately 1400 lines of code.
The
main reason I am insisting on this is that we'll want to modify the
UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just
leave
it to us. Your contribution is appreciated as it is.

   Then, storing default and user preferences for key bindings in a

  binary form makes it difficult to change them for no real reason.
  I've just seen that your lib already allows to store settings in a
  properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite

often
and not being able to do so by editing a text file would be a
problem.

The

advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI
perspective
(so options like 'next tab' and 'previous tab' are shown next to
each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by

manually
adding indexes. Here's a very rough example:

....
<impl>
<keybindings>
     <binding1>
         <actionName value="Previous Tab"/>
         ...
     </binding1>
     <binding2>
         <actionName value="Next Tab"/>
         ...
     </binding2>
</keybindings>
</impl>

   We can put the default ones into the defaults.properties files and

  then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll
look
into it.

Yes the defaults are new indeed.

   1. Would it be possible to implement support for alternative

  actions? Most key binding management UIs have it would be really
  nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

   2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,

  Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple
once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!
Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < >>>>>>>> emcho@sip-communicator.org >>>>>>>> >>>>>>>> <mailto:emcho@sip-communicator.org>> wrote:

  Hey Galen,

  Nice work indeed and the feature is quite useful. I've just tried
it
and
  I like it a lot. Thanks for contributing!

  There are however a few things that make me feel uncomfortable
with
the
  way this is currently integrated in SIP Communicator.

  First, I don't quite see the need of splitting the implementation
into a
  service and a plugin. The configuration UI could very well live in
  impl.keybinding. It would also be nice if we could copy necessary
  classes from your lib into the service implementation. There are
two
  reasons I believe this would a good idea:
         - We'd probably need to make changes on some parts such as
the
  configuration UI and the persistence mechanisms (see below) so
that
they
  would fit the SC conventions.
         - It would make the code easier to understand.

  Then, storing default and user preferences for key bindings in a
binary
  form makes it difficult to change them for no real reason. I've
just
  seen that your lib already allows to store settings in a
properties
file
  so I think this would fit better. We can put the default ones into
the
  defaults.properties files and then use the configuration service
to
  modify them.

  What do you think?

  Other than that I was wondering if I could make the following
feature
  requests:

  1.Would it be possible to implement support for alternative
actions?
  Most key binding management UIs have it would be really nice if we
  also did.
  and
  2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,
  Ctrl-u shortcuts that we've recently added to the chat window?

  Once again, thanks for contributing your work!

  Emil

  Yana Stamcheva написа:
  > Hi Galen,
  >
  > you've done a really great job in this patch!
  >
  > First let me apologize for making you wait so long, we were
really
  busy
  > lately. We have discussed the plugin off list already, but it's
now
  > applied and committed. You're also on our contributors page.
  >
  > (more inline)
  >
  > Damian Johnson wrote:
  >> Hi. I've just finished integrating a chooser for keybindings
into
  the SIP
  >> Communicator. This both provides a means for users to change
their
  >> keybinding preferences and save them between runs. Most of the
  details of
  >> the addition are described in the latest blog post
  >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
  >> two additions:
  >> 1. Keybinding persistence via a new service. This saves custom
  bindings with
  >> the user's preferences and informs any frames using the
bindings
  of updates.
  >> *src/net/java/sip/communicator/impl/keybindings
  >> src/net/java/sip/communicator/service/keybindings
  >
  > Applied. I like the way you have implemented key bindings in the
  gui bundle!
  >
  >> *
  >> 2. Plugin that provides a ConfigurationForm for changing the
  keybindings.
  >> *src/net/java/sip/communicator/plugin/keybindingChooser*
  >>
  >> I've added a how-to for adding new keybinding sets
  >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
the
  >> chooser description). Also, this is my first time making a
patch
  >> - are binaries supposed to be provided separately like this? I
  didn't spot
  >> an option to include them in svn diff...
  >
  > My reply is coming late, but yes binaries should be supplied
  separately,
  > because they're not contained in the diff. The how-to is also
very
  good
  > idea. Let me know if you're interested in adding this on the SIP
  > Communicator website.
  >
  >> A couple of usability features (any opinions?):
  >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
  >> 2. Keybindings aren't changed until the user hits "Apply".
  Perhaps bindings
  >> that haven't been applied should be discolored to draw
attention
  to them?
  >
  > My personal opinion is that a button "Defaults" could be really
  helpful.
  > Otherwise coloring unsaved bindings could be useful, but also it
could
  > be confusing, the user could not be sure what exactly the color
means.
  >
  >> Known Issue:
  >> Not all permissible bindings work in all situations. For
instance
  in the
  >> main frame the 'Contacts' and 'Chat rooms' panels appear to
  intercept some
  >> keyboard events, preventing bindings to directional arrows
(without
  >> modifiers) or the space bar from working. This isn't a terribly
  big woop but
  >> might possibly confuse users.
  >
  > Hm, could you explain more in details the problem situations, we
could
  > try to resolve this.
  >
  >> A few unrelated things I've come across:
  >> 1. The patch also includes the fix for a minor (but very
consistent)
  >> spelling error: 'desactivate' to 'deactivate'. This included
  tweaking an
  >> image with the word, changing some language files, and
  refactoring parts of
  >> the ManageButtonsPanel class.
  >
  > Committed all spelling corrections. Thanks.
  >
  >> 2. Does anyone know if there's a known issue with the
chatalerter
  plugin?
  >> Eclipse has complained that the org.jdesktop.jdic.misc package
  can't be
  >> resolved ever since I checked it out via svn. However this has
never
  >> prevented the SIP Communicator from running. I've tried asking
a
  couple of
  >> times on the IRC channel but I think I keep catching everyone
  when they're
  >> asleep...
  >
  > Maybe you've already figured it out yourself, but I'm just
  replying for
  > the record..Even that Eclipse is complaining about a library, if
you
  > compile and run SIP Communicator through Ant it will always
work,
  > because the classpath is set inside the build.xml. If Eclipse is
  > complaining about a library this means that this library is not
  added in
  > the .classpath file of Eclipse. You need to do the following:
right
  > click on the project / "Properties" / "Project Build Path" /
  "Libraries"
  > / "Add new" / Find the library in the lib directory of SComm.
  >
  >> 3. Couple of minor issues on the site:
  >> Developer Documentation > How to Configure Eclipse
  >> Minor simplification- it's not necessary to get the old version
  of Subclipse
  >> then upgrade it. The buckminster error can be avoided by
  disabling the
  >> optional components.
  >
  > Nice catch! I didn't experienced this error, so I'm not very
familiar
  > with the problem, so again are you interested in changing the
part
of
  > the tutorial concerning the problem.
  >
  >> Developer Documentation > UI Service > Create and add
  configuration forms
  >> As Atul noticed the 'getConfigurationManager()' method doesn't
  exist and
  >> should be 'getConfigurationWindow()'.
  >
  > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
  this one,
  > could you be more precise?
  >
  > Damian, the patch was very well documented, very mature and easy
to
  > read. Again you've done a very good work! Thank you for the
  patience and
  > keep up the good work:)
  >
  > Cheers,
  > Yana
  >
  >> Hope this helps. Cheers! -Galen
  >>
  >> PS. I'm planning to try going by my middle name, Galen, in SIP
  >> correspondence to avoid confusion with Damien and the other
  Damian. :slight_smile:
  >>
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
  >>

------------------------------------------------------------------------
  >>
  >>
---------------------------------------------------------------------
  >> To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >

  >
---------------------------------------------------------------------
  > To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >

  >

---------------------------------------------------------------------
  To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:

  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#10

Thanks Damien, good to know this is on the right track. It looks like the
best way of getting a set of related defaults is to snag all of them via
'getCurrentSettings()' then filter for what's needed. Introducing ordering
will be a bit hacky, but cest la vi. Cheers! -Damian

···

On Sat, Jul 26, 2008 at 9:36 PM, Damien Roth <damien.roth@gmail.com> wrote:

Hello Damian,

I've work on the configuration service (for my GSoC project) and every
data is created by the classes which want to store data. If the file
doesn't exists, the ConfigurationServiceImpl just call the method:
"createPropertiesDocument" which just create the root of the tree.

The best way, is to store the default data like you said in the
resources directory and when you bundle is running, checks if the data
are stored in the configuration file. If it's not the case, use a
method to add them by reading the key's in the default data file.

Bye

Damien

2008/7/27 Damian Johnson <atagar1@gmail.com>:
> Hi. I've tweaked the KeybindingsService implementation to utilize the
> ConfigurationService so the custom bindings are now stored in a
> human-editable fashion. However, I'm having some difficulty finding where
> the defaults should be stored. The obvious choice is
> 'resources/config/defaults.properties' but the ResourceManagementService,
> unlike ConfigurationService, isn't structured as a tree and hence doesn't
> have any methods for retrieving batches of defaults (ie, all chat
> keybindings). Per chance is there a file with the initial values of
> '~/.sip-communicator/sip-communicator.xml'? -Damian
>
> 2008/7/25 Damian Johnson <atagar1@gmail.com>
>>
>> Hi Yana. Thanks for the quick response! Previous discussion on this
thread
>> concerned the binary property files. I chose serialized linked hash maps
so
>> ordering would be preserved (which is important for the appearance).
Emil's
>> suggested adding an index to the plain text properties and I'm planning
to
>> look into this soon. -Damian
>>
>> 2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:
>>>
>>> Hi Damian,
>>>
>>> Damian Johnson wrote:
>>>>
>>>> Hi. Thus far I've made the following changes:
>>>> Added 'Set Default' button
>>>> Added bindings for fonts
>>>> Merged keybindingchoosr plugin into keybindings service implementation
>>>> Bug fix: Issue with the escape binding (thanks, Pavel, for catching
>>>> that!)
>>>> Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
>>>> NullPointerException if user cancels the color chooser
>>>> Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
>>>> 'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths
>>>
>>> Good job!
>>>
>>>>
>>>> However, I've come up against a bit of a chicken-and-egg problem. The
>>>> swing-ui bundle's activator uses the keybindings service to initialize
>>>> the
>>>> UI, hence that service needs to start first. However, the keybinding
>>>> chooser
>>>> needs the swing-ui to set up the custom look-and-feel (the
>>>> ConfigurationForm
>>>> is using the swing default now that it's starting first).
>>>>
>>>> I think we should merge the keybindings service into swing-ui (*
>>>> /impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
>>>> beyond
>>>> this issue:
>>>> 1. Currently the KeybindingsService is being provided through the
>>>> GuiActivator (part of the swing-ui bundle). Actually, the keybindings
>>>> service isn't available to plugins unless we're gonna make the service
>>>> available through something like the UIService.
>>>> 2. Keybindings are strictly a UI feature anyway…
>>>>
>>>> Does this sound good? If not, any thoughts on alternatives? Cheers!
>>>> -Damian
>>>
>>> Sounds very good. I have also discussed this with Emil off list and he
>>> also agrees, so you could go ahead and integrate it in the swing-ui.
>>>
>>> Just one more thing. I'm not sure if Emil has already asked, but is
there
>>> any special reason for which the configuration files for keybindings :
>>> keybindings-chat and keybindings-main are binary files? Could we make
them
>>> just properties files, so that they're easier to read?
>>>
>>> Cheers,
>>> Yana
>>>
>>>>
>>>> 2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:
>>>>
>>>>> Hi Pavel,
>>>>>
>>>>> I have committed a fix and the font button should be now working
(from
>>>>> build 1242). Let me know if you continue experiencing problems.
>>>>>
>>>>> Cheers,
>>>>> Yana
>>>>>
>>>>>
>>>>> Yana Stamcheva wrote:
>>>>>
>>>>>> Hi Pavel,
>>>>>>
>>>>>> thanks for reporting!
>>>>>>
>>>>>> I think that only the Escape key problem is related to Damian's
work,
>>>>>> but
>>>>>> I'm confirming the two other problems.
>>>>>>
>>>>>> I'll have a look at the font button asap.
>>>>>>
>>>>>> Is there someone else experiencing encoding problems when
>>>>>> sending/receiving messages ? The first thing that comes to me is
that
>>>>>> it
>>>>>> could be due to recent modifications for HTML message support.
>>>>>>
>>>>>> Cheers,
>>>>>> Yana
>>>>>>
>>>>>> Pavel Tankov wrote:
>>>>>>
>>>>>>> Hello Damian,
>>>>>>>
>>>>>>> It looks like there are a few problems now with the new keybinding
>>>>>>> plugin. Here are a few I've noticed that worked before, but now
>>>>>>> don't:
>>>>>>> - Escape key doesn't close chat window;
>>>>>>> - The button for selecting font doesn't popup the selection window;
>>>>>>> - My messages sent in cyrillic appear as ??? on the other side.
>>>>>>> Probably that's true for other non-latin encodings.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Pavel Tankov
>>>>>>>
>>>>>>> ----- Original Message ----
>>>>>>> From: Damian Johnson <atagar1@gmail.com>
>>>>>>> To: dev@sip-communicator.dev.java.net
>>>>>>> Sent: Saturday, July 19, 2008 9:22:57 AM
>>>>>>> Subject: Re: [sip-comm-dev] Keybinding Chooser Addition
>>>>>>>
>>>>>>>
>>>>>>> Hi Emil,
>>>>>>>
>>>>>>> I am not sure I see why this prevents us from copying the content
of
>>>>>>> the
>>>>>>> plugin.keybindingchooser package into impl.keybindings.
>>>>>>> Ack! Sorry, I misread your original comment and thought the
>>>>>>> proposition
>>>>>>> was to do away with the service. My bad.
>>>>>>> I was trying to say that since the configuration UI is quite
tightly
>>>>>>> coupled with this service implementation and both are quite light
>>>>>>> then they
>>>>>>> probably should belong to the same package… It would have made
>>>>>>> sense to
>>>>>>> keep them separated if there's a chance of someone needing
concurrent
>>>>>>> versions of the chooser UI, which I don't think is very probable.
>>>>>>> Good points- I'll make the change.
>>>>>>> Well unless I am missing something we'll probably only need what's
>>>>>>> in
>>>>>>> the chooser package which gives us approximately 1400 lines of
code.
>>>>>>> The
>>>>>>> main reason I am insisting on this is that we'll want to modify the
>>>>>>> UI and
>>>>>>> having the code in there would make it easier… If you don't feel
>>>>>>> like
>>>>>>> making the change then, by all means, just leave it to us. Your
>>>>>>> contribution
>>>>>>> is appreciated as it is
>>>>>>> As I mentioned if we're gonna make substantial changes to the UI
then
>>>>>>> this makes sense, and the 'alternative bindings' are certainly
>>>>>>> substantial
>>>>>>> enough. I'm sorry if I gave the impression that I wasn't willing to
>>>>>>> make the
>>>>>>> change- I'll handle it.
>>>>>>> … I meant alternative bindings
>>>>>>> I've never seen this feature before but should be simple once the
>>>>>>> chooser's source is added in.
>>>>>>> Hopefully we'll soon be able to start integrating the spell
checker
>>>>>>> soon
>>>>>>> too. :slight_smile:
>>>>>>> Cheers,
>>>>>>> Damian
>>>>>>> PS. I give up on going by 'Galen'. With webmail metadata adding in
>>>>>>> my
>>>>>>> name this is just confusing. Oh well- it was worth a try…
>>>>>>> On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov > >>>>>>> <emcho@sip-communicator.org> > >>>>>>> wrote:
>>>>>>>
>>>>>>> Hey Damian,
>>>>>>>
>>>>>>> Damian Johnson написа:
>>>>>>>
>>>>>>> First, I don't quite see the need of splitting the
implementation
>>>>>>>>
>>>>>>>> into a service and a plugin.
>>>>>>>>
>>>>>>>> Actually I was just thinking about this. Are we going to want
>>>>>>>> plugins to
>>>>>>>> be able to listen for and process keyboard events?
>>>>>>>>
>>>>>>> Most probably.
>>>>>>>
>>>>>>>
>>>>>>> If so then the
>>>>>>>>
>>>>>>>> addition shouldn't use the new methods provided via the
>>>>>>>> SIPCommFrame. If
>>>>>>>> not then you're completely right.
>>>>>>>>
>>>>>>> I am not sure I see why this prevents us from copying the content
of
>>>>>>> the
>>>>>>> plugin.keybindingchooser package into impl.keybindings.
>>>>>>>
>>>>>>> Guess I wasn't clear. Apologies. I was trying to say that since the
>>>>>>> configuration UI is quite tightly coupled with this service
>>>>>>> implementation and both are quite light then they probably should
>>>>>>> belong
>>>>>>> to the same package.
>>>>>>>
>>>>>>>
>>>>>>> The configuration UI could very well live in impl.keybinding.
>>>>>>>>
>>>>>>>> Certainly could. Assuming developers aren't interested in making
>>>>>>>> alternative choosers (which I doubt) there's no need for it to act
>>>>>>>> as a
>>>>>>>> plugin.
>>>>>>>>
>>>>>>> I agree, developers are likely to tackle the UI but since both the
>>>>>>> plugin and the impl are quite light, merging them would not make
>>>>>>> modifying the UI any more difficult. Quite, on the contrary, code
>>>>>>> would
>>>>>>> be easier to read since it would all be in the same place.
>>>>>>>
>>>>>>> It would have made sense to keep them separated if there's a chance
>>>>>>> of
>>>>>>> someone needing concurrent versions of the chooser UI, which I
don't
>>>>>>> think is very probable.
>>>>>>>
>>>>>>>
>>>>>>> It would also be nice if we could copy necessary classes from
your
>>>>>>>>
>>>>>>>> lib into the service implementation.
>>>>>>>>
>>>>>>>> It would be easy to do- autoformatting is a beautiful thing so
>>>>>>>> getting
>>>>>>>> it to conform to the coding guidelines would be a breeze. However,
>>>>>>>> unless we're gonna make radical changes to the UI I don't think
this
>>>>>>>> is
>>>>>>>> a good idea. Here's a couple of issues:
>>>>>>>>
>>>>>>>> 1. As the how-to mentions the jar is executable, providing an
editor
>>>>>>>> to
>>>>>>>> easily tweak the keybindings. If integrated we'd still probably
need
>>>>>>>> the
>>>>>>>> jar for that functionality (with the exception of the Persistence
>>>>>>>> class
>>>>>>>> that package probably woulnd't be integrated).
>>>>>>>>
>>>>>>> I was actually thinking that if we start saving everything in text
>>>>>>> form
>>>>>>> then we won't need to use the standalone tool when generating or
>>>>>>> modifying the default settings.
>>>>>>>
>>>>>>>
>>>>>>> 2. As far as hacks go, what's used is pretty short and sweet
>>>>>>> (providing
>>>>>>>>
>>>>>>>> an I18N label abstraction and stretching the layout a bit). I
doubt
>>>>>>>> breaking open the jar would clarify anything- it would certainly
>>>>>>>> give a
>>>>>>>> lot more code to look at.
>>>>>>>>
>>>>>>> Well unless I am missing something we'll probably only need what's
in
>>>>>>> the chooser package which gives us approximately 1400 lines of
code.
>>>>>>> The
>>>>>>> main reason I am insisting on this is that we'll want to modify the
>>>>>>> UI
>>>>>>> and having the code in there would make it easier.
>>>>>>>
>>>>>>> If you don't feel like making the change then, by all means, just
>>>>>>> leave
>>>>>>> it to us. Your contribution is appreciated as it is.
>>>>>>>
>>>>>>>
>>>>>>> Then, storing default and user preferences for key bindings in a
>>>>>>>>
>>>>>>>> binary form makes it difficult to change them for no real
reason.
>>>>>>>> I've just seen that your lib already allows to store settings in
a
>>>>>>>> properties file so I think this would fit better.
>>>>>>>>
>>>>>>>> I actually spent quite a while antagonizing over this point.
>>>>>>>>
>>>>>>> The point is that we will probably need to modify defaults quite
>>>>>>> often
>>>>>>> and not being able to do so by editing a text file would be a
>>>>>>> problem.
>>>>>>>
>>>>>>>
>>>>>>> The
>>>>>>>>
>>>>>>>> advantage of storing it as a serialized linked hash map is that it
>>>>>>>> retains ordering information, which is important from a UI
>>>>>>>> perspective
>>>>>>>> (so options like 'next tab' and 'previous tab' are shown next to
>>>>>>>> each
>>>>>>>> other). If that wasn't a concern then certainly- viva la plain
text.
>>>>>>>>
>>>>>>> You can easily implement this with our ConfigurationService by
>>>>>>> manually
>>>>>>> adding indexes. Here's a very rough example:
>>>>>>>
>>>>>>> …
>>>>>>> <impl>
>>>>>>> <keybindings>
>>>>>>> <binding1>
>>>>>>> <actionName value="Previous Tab"/>
>>>>>>> …
>>>>>>> </binding1>
>>>>>>> <binding2>
>>>>>>> <actionName value="Next Tab"/>
>>>>>>> …
>>>>>>> </binding2>
>>>>>>> </keybindings>
>>>>>>> </impl>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> We can put the default ones into the defaults.properties files
and
>>>>>>>>
>>>>>>>> then use the configuration service to modify them.
>>>>>>>>
>>>>>>>> It looks like this service has changed quite a bit recently. I'll
>>>>>>>> look
>>>>>>>> into it.
>>>>>>>>
>>>>>>> Yes the defaults are new indeed.
>>>>>>>
>>>>>>>
>>>>>>> 1. Would it be possible to implement support for alternative
>>>>>>>>
>>>>>>>> actions? Most key binding management UIs have it would be really
>>>>>>>> nice if we also did.
>>>>>>>>
>>>>>>>> I'm not familiar with this. Alternative action of what?
>>>>>>>>
>>>>>>> Sorry, my bad, I meant alternative bindings. (See attached PNG)
>>>>>>>
>>>>>>>
>>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>>>>>>> Ctrl-i,
>>>>>>>>
>>>>>>>> Ctrl-u shortcuts that we've recently added to the chat window?
>>>>>>>>
>>>>>>>> Oops! That's what I get for failing to sync with the trunk. Simple
>>>>>>>> once
>>>>>>>> I get to a beefier connection.
>>>>>>>>
>>>>>>> Thanks, I was hoping you'd say so :wink:
>>>>>>>
>>>>>>>
>>>>>>> Thanks again for the feedback!
>>>>>>> Thank you Damian, I do like the feature very much!
>>>>>>>
>>>>>>> Cheers
>>>>>>> Emil
>>>>>>>
>>>>>>> Cheers,
>>>>>>>>
>>>>>>>> Galen
>>>>>>>>
>>>>>>>> On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov > >>>>>>>> <emcho@sip-communicator.org > >>>>>>>> > >>>>>>> <mailto:emcho@sip-communicator.org>> wrote:
>>>>>>>>
>>>>>>>> Hey Galen,
>>>>>>>>
>>>>>>>> Nice work indeed and the feature is quite useful. I've just
tried
>>>>>>>> it
>>>>>>>> and
>>>>>>>> I like it a lot. Thanks for contributing!
>>>>>>>>
>>>>>>>> There are however a few things that make me feel uncomfortable
>>>>>>>> with
>>>>>>>> the
>>>>>>>> way this is currently integrated in SIP Communicator.
>>>>>>>>
>>>>>>>> First, I don't quite see the need of splitting the
implementation
>>>>>>>> into a
>>>>>>>> service and a plugin. The configuration UI could very well live
in
>>>>>>>> impl.keybinding. It would also be nice if we could copy
necessary
>>>>>>>> classes from your lib into the service implementation. There are
>>>>>>>> two
>>>>>>>> reasons I believe this would a good idea:
>>>>>>>> - We'd probably need to make changes on some parts such
as
>>>>>>>> the
>>>>>>>> configuration UI and the persistence mechanisms (see below) so
>>>>>>>> that
>>>>>>>> they
>>>>>>>> would fit the SC conventions.
>>>>>>>> - It would make the code easier to understand.
>>>>>>>>
>>>>>>>> Then, storing default and user preferences for key bindings in a
>>>>>>>> binary
>>>>>>>> form makes it difficult to change them for no real reason. I've
>>>>>>>> just
>>>>>>>> seen that your lib already allows to store settings in a
>>>>>>>> properties
>>>>>>>> file
>>>>>>>> so I think this would fit better. We can put the default ones
into
>>>>>>>> the
>>>>>>>> defaults.properties files and then use the configuration service
>>>>>>>> to
>>>>>>>> modify them.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> Other than that I was wondering if I could make the following
>>>>>>>> feature
>>>>>>>> requests:
>>>>>>>>
>>>>>>>> 1.Would it be possible to implement support for alternative
>>>>>>>> actions?
>>>>>>>> Most key binding management UIs have it would be really nice if
we
>>>>>>>> also did.
>>>>>>>> and
>>>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>>>>>>>> Ctrl-i,
>>>>>>>> Ctrl-u shortcuts that we've recently added to the chat window?
>>>>>>>>
>>>>>>>> Once again, thanks for contributing your work!
>>>>>>>>
>>>>>>>> Emil
>>>>>>>>
>>>>>>>>
>>>>>>>> Yana Stamcheva написа:
>>>>>>>> > Hi Galen,
>>>>>>>> >
>>>>>>>> > you've done a really great job in this patch!
>>>>>>>> >
>>>>>>>> > First let me apologize for making you wait so long, we were
>>>>>>>> really
>>>>>>>> busy
>>>>>>>> > lately. We have discussed the plugin off list already, but
it's
>>>>>>>> now
>>>>>>>> > applied and committed. You're also on our contributors page.
>>>>>>>> >
>>>>>>>> > (more inline)
>>>>>>>> >
>>>>>>>> > Damian Johnson wrote:
>>>>>>>> >> Hi. I've just finished integrating a chooser for keybindings
>>>>>>>> into
>>>>>>>> the SIP
>>>>>>>> >> Communicator. This both provides a means for users to change
>>>>>>>> their
>>>>>>>> >> keybinding preferences and save them between runs. Most of
the
>>>>>>>> details of
>>>>>>>> >> the addition are described in the latest blog post
>>>>>>>> >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
>>>>>>>> consists of
>>>>>>>> >> two additions:
>>>>>>>> >> 1. Keybinding persistence via a new service. This saves
custom
>>>>>>>> bindings with
>>>>>>>> >> the user's preferences and informs any frames using the
>>>>>>>> bindings
>>>>>>>> of updates.
>>>>>>>> >> *src/net/java/sip/communicator/impl/keybindings
>>>>>>>> >> src/net/java/sip/communicator/service/keybindings
>>>>>>>> >
>>>>>>>> > Applied. I like the way you have implemented key bindings in
the
>>>>>>>> gui bundle!
>>>>>>>> >
>>>>>>>> >> *
>>>>>>>> >> 2. Plugin that provides a ConfigurationForm for changing the
>>>>>>>> keybindings.
>>>>>>>> >> *src/net/java/sip/communicator/plugin/keybindingChooser*
>>>>>>>> >>
>>>>>>>> >> I've added a how-to for adding new keybinding sets
>>>>>>>> >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php
>(below
>>>>>>>> the
>>>>>>>> >> chooser description). Also, this is my first time making a
>>>>>>>> patch
>>>>>>>> >> - are binaries supposed to be provided separately like this?
I
>>>>>>>> didn't spot
>>>>>>>> >> an option to include them in svn diff…
>>>>>>>> >
>>>>>>>> > My reply is coming late, but yes binaries should be supplied
>>>>>>>> separately,
>>>>>>>> > because they're not contained in the diff. The how-to is also
>>>>>>>> very
>>>>>>>> good
>>>>>>>> > idea. Let me know if you're interested in adding this on the
SIP
>>>>>>>> > Communicator website.
>>>>>>>> >
>>>>>>>> >> A couple of usability features (any opinions?):
>>>>>>>> >> 1. Add a key to revert bindings to defaults. Helpful or
>>>>>>>> unnecessary?
>>>>>>>> >> 2. Keybindings aren't changed until the user hits "Apply".
>>>>>>>> Perhaps bindings
>>>>>>>> >> that haven't been applied should be discolored to draw
>>>>>>>> attention
>>>>>>>> to them?
>>>>>>>> >
>>>>>>>> > My personal opinion is that a button "Defaults" could be
really
>>>>>>>> helpful.
>>>>>>>> > Otherwise coloring unsaved bindings could be useful, but also
it
>>>>>>>> could
>>>>>>>> > be confusing, the user could not be sure what exactly the
color
>>>>>>>> means.
>>>>>>>> >
>>>>>>>> >> Known Issue:
>>>>>>>> >> Not all permissible bindings work in all situations. For
>>>>>>>> instance
>>>>>>>> in the
>>>>>>>> >> main frame the 'Contacts' and 'Chat rooms' panels appear to
>>>>>>>> intercept some
>>>>>>>> >> keyboard events, preventing bindings to directional arrows
>>>>>>>> (without
>>>>>>>> >> modifiers) or the space bar from working. This isn't a
terribly
>>>>>>>> big woop but
>>>>>>>> >> might possibly confuse users.
>>>>>>>> >
>>>>>>>> > Hm, could you explain more in details the problem situations,
we
>>>>>>>> could
>>>>>>>> > try to resolve this.
>>>>>>>> >
>>>>>>>> >> A few unrelated things I've come across:
>>>>>>>> >> 1. The patch also includes the fix for a minor (but very
>>>>>>>> consistent)
>>>>>>>> >> spelling error: 'desactivate' to 'deactivate'. This included
>>>>>>>> tweaking an
>>>>>>>> >> image with the word, changing some language files, and
>>>>>>>> refactoring parts of
>>>>>>>> >> the ManageButtonsPanel class.
>>>>>>>> >
>>>>>>>> > Committed all spelling corrections. Thanks.
>>>>>>>> >
>>>>>>>> >> 2. Does anyone know if there's a known issue with the
>>>>>>>> chatalerter
>>>>>>>> plugin?
>>>>>>>> >> Eclipse has complained that the org.jdesktop.jdic.misc
package
>>>>>>>> can't be
>>>>>>>> >> resolved ever since I checked it out via svn. However this
has
>>>>>>>> never
>>>>>>>> >> prevented the SIP Communicator from running. I've tried
asking
>>>>>>>> a
>>>>>>>> couple of
>>>>>>>> >> times on the IRC channel but I think I keep catching everyone
>>>>>>>> when they're
>>>>>>>> >> asleep…
>>>>>>>> >
>>>>>>>> > Maybe you've already figured it out yourself, but I'm just
>>>>>>>> replying for
>>>>>>>> > the record…Even that Eclipse is complaining about a library,
if
>>>>>>>> you
>>>>>>>> > compile and run SIP Communicator through Ant it will always
>>>>>>>> work,
>>>>>>>> > because the classpath is set inside the build.xml. If Eclipse
is
>>>>>>>> > complaining about a library this means that this library is
not
>>>>>>>> added in
>>>>>>>> > the .classpath file of Eclipse. You need to do the following:
>>>>>>>> right
>>>>>>>> > click on the project / "Properties" / "Project Build Path" /
>>>>>>>> "Libraries"
>>>>>>>> > / "Add new" / Find the library in the lib directory of SComm.
>>>>>>>> >
>>>>>>>> >> 3. Couple of minor issues on the site:
>>>>>>>> >> Developer Documentation > How to Configure Eclipse
>>>>>>>> >> Minor simplification- it's not necessary to get the old
version
>>>>>>>> of Subclipse
>>>>>>>> >> then upgrade it. The buckminster error can be avoided by
>>>>>>>> disabling the
>>>>>>>> >> optional components.
>>>>>>>> >
>>>>>>>> > Nice catch! I didn't experienced this error, so I'm not very
>>>>>>>> familiar
>>>>>>>> > with the problem, so again are you interested in changing the
>>>>>>>> part
>>>>>>>> of
>>>>>>>> > the tutorial concerning the problem.
>>>>>>>> >
>>>>>>>> >> Developer Documentation > UI Service > Create and add
>>>>>>>> configuration forms
>>>>>>>> >> As Atul noticed the 'getConfigurationManager()' method
doesn't
>>>>>>>> exist and
>>>>>>>> >> should be 'getConfigurationWindow()'.
>>>>>>>> >
>>>>>>>> > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
>>>>>>>> this one,
>>>>>>>> > could you be more precise?
>>>>>>>> >
>>>>>>>> > Damian, the patch was very well documented, very mature and
easy
>>>>>>>> to
>>>>>>>> > read. Again you've done a very good work! Thank you for the
>>>>>>>> patience and
>>>>>>>> > keep up the good work:)
>>>>>>>> >
>>>>>>>> > Cheers,
>>>>>>>> > Yana
>>>>>>>> >
>>>>>>>> >> Hope this helps. Cheers! -Galen
>>>>>>>> >>
>>>>>>>> >> PS. I'm planning to try going by my middle name, Galen, in
SIP
>>>>>>>> >> correspondence to avoid confusion with Damien and the other
>>>>>>>> Damian. :slight_smile:
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> >> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>> >> For additional commands, e-mail:
>>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>> >
>>>>>>>>
>>>>>>>> >
>>>>>>>>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> > To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>> > For additional commands, e-mail:
>>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>> >
>>>>>>>>
>>>>>>>> >
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>> For additional commands, e-mail:
>>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
------------------------------------------------------------------------
>>>>>>>>
>>>>>>>>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> For additional commands, e-mail:
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>>
>>>>>>>
---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>> For additional commands, e-mail:
>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>>>>>> For additional commands, e-mail:
>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>
>>>>>>
>>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>>>>> For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
>>>>>
>>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
>>> For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
>>>
>>
>
>

--
Damien ROTH
Programmeur n.m : Celui qui résout un problème que vous n'aviez pas,
d'une façon que vous ne comprenez pas.


#11

Hi. I've just finished implementing all the requested changes for the
keybindings service. In addition to the previous change list there's now
alternative binding support (see attached screenshot), the jar's been
unpacked (and removed) and the service no longer uses binaries for default
or custom bindings. The following example uses the bindings added for the
chat toolbar.

In defaults.properties the binding information looks like:
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.index=0
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcut=ctrl
pressed B
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcutAlt=ctrl
pressed F
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.index=1
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcut=ctrl
pressed I
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcutAlt=ctrl
pressed K
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.index=2
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.shortcut=ctrl
pressed U

Field's functions should be self explanatory and the alternate shortcuts
('shortcutAlt') are optional. Custom bindings are stored via the
ConfigurationService (in my case to
~/.sip-communicator/sip-communicator.xml) and look something like:
<keybindings>
  <chatToolbar>
    <bold value="ctrl pressed B"/>
    <bold-alt value="ctrl pressed F"/>
    <italic value="ctrl pressed I"/>
    <italic-alt value="ctrl pressed K"/>
    <underline value="ctrl pressed U"/>
    <underline-alt value=""/>
  </chatToolbar>
</keybindings>

Quite a bit was removed and I'm not sure if the patch reflects that so I've
included the svn status output to give an indication of where changes were
made. Unlike the normal bindings, the alternatives can be removed (with the
escape key). One other note is that I added an
'isSettingsStringAvailable(String)' method to the ResourceManagementService
since otherwise there's no means of figuring out if a default exists.
Personally I think the getSettingsString(String) method should throw the
MissingResourceException rather than log errors (and have a javadoc
description) but I'm not sure if changing this would upset other classes
that currently use it. Cheers! -Damian

keybindingsUpdate.patch (173 KB)

svnStatusOutput.txt (3.85 KB)

···

2008/7/27 Emil Ivov <emcho@sip-communicator.org>

Hey Damian,

Glad to hear you are making progress on this one!

The defaults.properties file was indeed previewed for cases just like
yours: the conf service is supposed to load it as default content the
first time it runs.

However, as you have noticed this behaviour hasn't been fully
implemented yet so right now you can either implement it yourself or
do as Damien suggested - use a resource file inside your bundle and in
case your properties are not already in the ConfigurationService, load
them in there by your self.

Cheers
Emil

On 7/27/08, Damian Johnson <atagar1@gmail.com> wrote:
> Hi. I've tweaked the KeybindingsService implementation to utilize the
> ConfigurationService so the custom bindings are now stored in a
> human-editable fashion. However, I'm having some difficulty finding where
> the defaults should be stored. The obvious choice is
> 'resources/config/defaults.properties' but the ResourceManagementService,
> unlike ConfigurationService, isn't structured as a tree and hence doesn't
> have any methods for retrieving batches of defaults (ie, all chat
> keybindings). Per chance is there a file with the initial values of
> '~/.sip-communicator/sip-communicator.xml'? -Damian
>
> 2008/7/25 Damian Johnson <atagar1@gmail.com>
>
>> Hi Yana. Thanks for the quick response! Previous discussion on this
thread
>> concerned the binary property files. I chose serialized linked hash maps
>> so
>> ordering would be preserved (which is important for the appearance).
>> Emil's
>> suggested adding an index to the plain text properties and I'm planning
to
>> look into this soon. -Damian
>>
>> 2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:
>>
>> Hi Damian,
>>>
>>> Damian Johnson wrote:
>>>
>>>> Hi. Thus far I've made the following changes:
>>>> Added 'Set Default' button
>>>> Added bindings for fonts
>>>> Merged keybindingchoosr plugin into keybindings service implementation
>>>> Bug fix: Issue with the escape binding (thanks, Pavel, for catching
>>>> that!)
>>>> Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
>>>> NullPointerException if user cancels the color chooser
>>>> Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
>>>> 'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths
>>>>
>>>
>>> Good job!
>>>
>>>
>>>> However, I've come up against a bit of a chicken-and-egg problem. The
>>>> swing-ui bundle's activator uses the keybindings service to initialize
>>>> the
>>>> UI, hence that service needs to start first. However, the keybinding
>>>> chooser
>>>> needs the swing-ui to set up the custom look-and-feel (the
>>>> ConfigurationForm
>>>> is using the swing default now that it's starting first).
>>>>
>>>> I think we should merge the keybindings service into swing-ui (*
>>>> /impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
>>>> beyond
>>>> this issue:
>>>> 1. Currently the KeybindingsService is being provided through the
>>>> GuiActivator (part of the swing-ui bundle). Actually, the keybindings
>>>> service isn't available to plugins unless we're gonna make the service
>>>> available through something like the UIService.
>>>> 2. Keybindings are strictly a UI feature anyway…
>>>>
>>>> Does this sound good? If not, any thoughts on alternatives? Cheers!
>>>> -Damian
>>>>
>>>
>>> Sounds very good. I have also discussed this with Emil off list and he
>>> also agrees, so you could go ahead and integrate it in the swing-ui.
>>>
>>> Just one more thing. I'm not sure if Emil has already asked, but is
there
>>> any special reason for which the configuration files for keybindings :
>>> keybindings-chat and keybindings-main are binary files? Could we make
>>> them
>>> just properties files, so that they're easier to read?
>>>
>>> Cheers,
>>> Yana
>>>
>>>
>>>
>>>> 2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:
>>>>
>>>> Hi Pavel,
>>>>>
>>>>> I have committed a fix and the font button should be now working
(from
>>>>> build 1242). Let me know if you continue experiencing problems.
>>>>>
>>>>> Cheers,
>>>>> Yana
>>>>>
>>>>>
>>>>> Yana Stamcheva wrote:
>>>>>
>>>>> Hi Pavel,
>>>>>>
>>>>>> thanks for reporting!
>>>>>>
>>>>>> I think that only the Escape key problem is related to Damian's
work,
>>>>>> but
>>>>>> I'm confirming the two other problems.
>>>>>>
>>>>>> I'll have a look at the font button asap.
>>>>>>
>>>>>> Is there someone else experiencing encoding problems when
>>>>>> sending/receiving messages ? The first thing that comes to me is
that
>>>>>> it
>>>>>> could be due to recent modifications for HTML message support.
>>>>>>
>>>>>> Cheers,
>>>>>> Yana
>>>>>>
>>>>>> Pavel Tankov wrote:
>>>>>>
>>>>>> Hello Damian,
>>>>>>>
>>>>>>> It looks like there are a few problems now with the new keybinding
>>>>>>> plugin. Here are a few I've noticed that worked before, but now
>>>>>>> don't:
>>>>>>> - Escape key doesn't close chat window;
>>>>>>> - The button for selecting font doesn't popup the selection window;
>>>>>>> - My messages sent in cyrillic appear as ??? on the other side.
>>>>>>> Probably that's true for other non-latin encodings.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Pavel Tankov
>>>>>>>
>>>>>>> ----- Original Message ----
>>>>>>> From: Damian Johnson <atagar1@gmail.com>
>>>>>>> To: dev@sip-communicator.dev.java.net
>>>>>>> Sent: Saturday, July 19, 2008 9:22:57 AM
>>>>>>> Subject: Re: [sip-comm-dev] Keybinding Chooser Addition
>>>>>>>
>>>>>>>
>>>>>>> Hi Emil,
>>>>>>>
>>>>>>> I am not sure I see why this prevents us from copying the content
of
>>>>>>> the
>>>>>>> plugin.keybindingchooser package into impl.keybindings.
>>>>>>> Ack! Sorry, I misread your original comment and thought the
>>>>>>> proposition
>>>>>>> was to do away with the service. My bad.
>>>>>>> I was trying to say that since the configuration UI is quite
tightly
>>>>>>> coupled with this service implementation and both are quite light
>>>>>>> then
>>>>>>> they
>>>>>>> probably should belong to the same package… It would have made
>>>>>>> sense to
>>>>>>> keep them separated if there's a chance of someone needing
concurrent
>>>>>>> versions of the chooser UI, which I don't think is very probable.
>>>>>>> Good points- I'll make the change.
>>>>>>> Well unless I am missing something we'll probably only need what's
>>>>>>> in
>>>>>>> the chooser package which gives us approximately 1400 lines of
code.
>>>>>>> The
>>>>>>> main reason I am insisting on this is that we'll want to modify the
>>>>>>> UI
>>>>>>> and
>>>>>>> having the code in there would make it easier… If you don't feel
>>>>>>> like
>>>>>>> making the change then, by all means, just leave it to us. Your
>>>>>>> contribution
>>>>>>> is appreciated as it is
>>>>>>> As I mentioned if we're gonna make substantial changes to the UI
then
>>>>>>> this makes sense, and the 'alternative bindings' are certainly
>>>>>>> substantial
>>>>>>> enough. I'm sorry if I gave the impression that I wasn't willing to
>>>>>>> make the
>>>>>>> change- I'll handle it.
>>>>>>> … I meant alternative bindings
>>>>>>> I've never seen this feature before but should be simple once the
>>>>>>> chooser's source is added in.
>>>>>>> Hopefully we'll soon be able to start integrating the spell
checker
>>>>>>> soon
>>>>>>> too. :slight_smile:
>>>>>>> Cheers,
>>>>>>> Damian
>>>>>>> PS. I give up on going by 'Galen'. With webmail metadata adding in
>>>>>>> my
>>>>>>> name this is just confusing. Oh well- it was worth a try…
>>>>>>> On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < > >>>>>>> emcho@sip-communicator.org> > >>>>>>> wrote:
>>>>>>>
>>>>>>> Hey Damian,
>>>>>>>
>>>>>>> Damian Johnson написа:
>>>>>>>
>>>>>>> First, I don't quite see the need of splitting the
implementation
>>>>>>>
>>>>>>>> into a service and a plugin.
>>>>>>>>
>>>>>>>> Actually I was just thinking about this. Are we going to want
>>>>>>>> plugins
>>>>>>>> to
>>>>>>>> be able to listen for and process keyboard events?
>>>>>>>>
>>>>>>>> Most probably.
>>>>>>>
>>>>>>>
>>>>>>> If so then the
>>>>>>>
>>>>>>>> addition shouldn't use the new methods provided via the
>>>>>>>> SIPCommFrame.
>>>>>>>> If
>>>>>>>> not then you're completely right.
>>>>>>>>
>>>>>>>> I am not sure I see why this prevents us from copying the content
>>>>>>>> of
>>>>>>> the
>>>>>>> plugin.keybindingchooser package into impl.keybindings.
>>>>>>>
>>>>>>> Guess I wasn't clear. Apologies. I was trying to say that since the
>>>>>>> configuration UI is quite tightly coupled with this service
>>>>>>> implementation and both are quite light then they probably should
>>>>>>> belong
>>>>>>> to the same package.
>>>>>>>
>>>>>>>
>>>>>>> The configuration UI could very well live in impl.keybinding.
>>>>>>>
>>>>>>>> Certainly could. Assuming developers aren't interested in making
>>>>>>>> alternative choosers (which I doubt) there's no need for it to act
>>>>>>>> as
>>>>>>>> a
>>>>>>>> plugin.
>>>>>>>>
>>>>>>>> I agree, developers are likely to tackle the UI but since both
the
>>>>>>> plugin and the impl are quite light, merging them would not make
>>>>>>> modifying the UI any more difficult. Quite, on the contrary, code
>>>>>>> would
>>>>>>> be easier to read since it would all be in the same place.
>>>>>>>
>>>>>>> It would have made sense to keep them separated if there's a chance
>>>>>>> of
>>>>>>> someone needing concurrent versions of the chooser UI, which I
don't
>>>>>>> think is very probable.
>>>>>>>
>>>>>>>
>>>>>>> It would also be nice if we could copy necessary classes from
your
>>>>>>>
>>>>>>>> lib into the service implementation.
>>>>>>>>
>>>>>>>> It would be easy to do- autoformatting is a beautiful thing so
>>>>>>>> getting
>>>>>>>> it to conform to the coding guidelines would be a breeze. However,
>>>>>>>> unless we're gonna make radical changes to the UI I don't think
this
>>>>>>>> is
>>>>>>>> a good idea. Here's a couple of issues:
>>>>>>>>
>>>>>>>> 1. As the how-to mentions the jar is executable, providing an
editor
>>>>>>>> to
>>>>>>>> easily tweak the keybindings. If integrated we'd still probably
need
>>>>>>>> the
>>>>>>>> jar for that functionality (with the exception of the Persistence
>>>>>>>> class
>>>>>>>> that package probably woulnd't be integrated).
>>>>>>>>
>>>>>>>> I was actually thinking that if we start saving everything in
text
>>>>>>> form
>>>>>>> then we won't need to use the standalone tool when generating or
>>>>>>> modifying the default settings.
>>>>>>>
>>>>>>>
>>>>>>> 2. As far as hacks go, what's used is pretty short and sweet
>>>>>>> (providing
>>>>>>>
>>>>>>>> an I18N label abstraction and stretching the layout a bit). I
doubt
>>>>>>>> breaking open the jar would clarify anything- it would certainly
>>>>>>>> give
>>>>>>>> a
>>>>>>>> lot more code to look at.
>>>>>>>>
>>>>>>>> Well unless I am missing something we'll probably only need
what's
>>>>>>> in
>>>>>>> the chooser package which gives us approximately 1400 lines of
code.
>>>>>>> The
>>>>>>> main reason I am insisting on this is that we'll want to modify the
>>>>>>> UI
>>>>>>> and having the code in there would make it easier.
>>>>>>>
>>>>>>> If you don't feel like making the change then, by all means, just
>>>>>>> leave
>>>>>>> it to us. Your contribution is appreciated as it is.
>>>>>>>
>>>>>>>
>>>>>>> Then, storing default and user preferences for key bindings in a
>>>>>>>
>>>>>>>> binary form makes it difficult to change them for no real
reason.
>>>>>>>> I've just seen that your lib already allows to store settings in
a
>>>>>>>> properties file so I think this would fit better.
>>>>>>>>
>>>>>>>> I actually spent quite a while antagonizing over this point.
>>>>>>>>
>>>>>>>> The point is that we will probably need to modify defaults quite
>>>>>>> often
>>>>>>> and not being able to do so by editing a text file would be a
>>>>>>> problem.
>>>>>>>
>>>>>>>
>>>>>>> The
>>>>>>>
>>>>>>>> advantage of storing it as a serialized linked hash map is that it
>>>>>>>> retains ordering information, which is important from a UI
>>>>>>>> perspective
>>>>>>>> (so options like 'next tab' and 'previous tab' are shown next to
>>>>>>>> each
>>>>>>>> other). If that wasn't a concern then certainly- viva la plain
text.
>>>>>>>>
>>>>>>>> You can easily implement this with our ConfigurationService by
>>>>>>> manually
>>>>>>> adding indexes. Here's a very rough example:
>>>>>>>
>>>>>>> …
>>>>>>> <impl>
>>>>>>> <keybindings>
>>>>>>> <binding1>
>>>>>>> <actionName value="Previous Tab"/>
>>>>>>> …
>>>>>>> </binding1>
>>>>>>> <binding2>
>>>>>>> <actionName value="Next Tab"/>
>>>>>>> …
>>>>>>> </binding2>
>>>>>>> </keybindings>
>>>>>>> </impl>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> We can put the default ones into the defaults.properties files
and
>>>>>>>
>>>>>>>> then use the configuration service to modify them.
>>>>>>>>
>>>>>>>> It looks like this service has changed quite a bit recently. I'll
>>>>>>>> look
>>>>>>>> into it.
>>>>>>>>
>>>>>>>> Yes the defaults are new indeed.
>>>>>>>
>>>>>>>
>>>>>>> 1. Would it be possible to implement support for alternative
>>>>>>>
>>>>>>>> actions? Most key binding management UIs have it would be really
>>>>>>>> nice if we also did.
>>>>>>>>
>>>>>>>> I'm not familiar with this. Alternative action of what?
>>>>>>>>
>>>>>>>> Sorry, my bad, I meant alternative bindings. (See attached PNG)
>>>>>>>
>>>>>>>
>>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>>>>>>> Ctrl-i,
>>>>>>>
>>>>>>>> Ctrl-u shortcuts that we've recently added to the chat window?
>>>>>>>>
>>>>>>>> Oops! That's what I get for failing to sync with the trunk. Simple
>>>>>>>> once
>>>>>>>> I get to a beefier connection.
>>>>>>>>
>>>>>>>> Thanks, I was hoping you'd say so :wink:
>>>>>>>
>>>>>>>
>>>>>>> Thanks again for the feedback!
>>>>>>> Thank you Damian, I do like the feature very much!
>>>>>>>
>>>>>>> Cheers
>>>>>>> Emil
>>>>>>>
>>>>>>> Cheers,
>>>>>>>
>>>>>>>> Galen
>>>>>>>>
>>>>>>>> On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < > >>>>>>>> emcho@sip-communicator.org > >>>>>>>> > >>>>>>>> <mailto:emcho@sip-communicator.org>> wrote:
>>>>>>>
>>>>>>>> Hey Galen,
>>>>>>>>
>>>>>>>> Nice work indeed and the feature is quite useful. I've just
tried
>>>>>>>> it
>>>>>>>> and
>>>>>>>> I like it a lot. Thanks for contributing!
>>>>>>>>
>>>>>>>> There are however a few things that make me feel uncomfortable
>>>>>>>> with
>>>>>>>> the
>>>>>>>> way this is currently integrated in SIP Communicator.
>>>>>>>>
>>>>>>>> First, I don't quite see the need of splitting the
implementation
>>>>>>>> into a
>>>>>>>> service and a plugin. The configuration UI could very well live
in
>>>>>>>> impl.keybinding. It would also be nice if we could copy
necessary
>>>>>>>> classes from your lib into the service implementation. There are
>>>>>>>> two
>>>>>>>> reasons I believe this would a good idea:
>>>>>>>> - We'd probably need to make changes on some parts such
as
>>>>>>>> the
>>>>>>>> configuration UI and the persistence mechanisms (see below) so
>>>>>>>> that
>>>>>>>> they
>>>>>>>> would fit the SC conventions.
>>>>>>>> - It would make the code easier to understand.
>>>>>>>>
>>>>>>>> Then, storing default and user preferences for key bindings in a
>>>>>>>> binary
>>>>>>>> form makes it difficult to change them for no real reason. I've
>>>>>>>> just
>>>>>>>> seen that your lib already allows to store settings in a
>>>>>>>> properties
>>>>>>>> file
>>>>>>>> so I think this would fit better. We can put the default ones
into
>>>>>>>> the
>>>>>>>> defaults.properties files and then use the configuration service
>>>>>>>> to
>>>>>>>> modify them.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> Other than that I was wondering if I could make the following
>>>>>>>> feature
>>>>>>>> requests:
>>>>>>>>
>>>>>>>> 1.Would it be possible to implement support for alternative
>>>>>>>> actions?
>>>>>>>> Most key binding management UIs have it would be really nice if
we
>>>>>>>> also did.
>>>>>>>> and
>>>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>>>>>>>> Ctrl-i,
>>>>>>>> Ctrl-u shortcuts that we've recently added to the chat window?
>>>>>>>>
>>>>>>>> Once again, thanks for contributing your work!
>>>>>>>>
>>>>>>>> Emil
>>>>>>>>
>>>>>>>>
>>>>>>>> Yana Stamcheva написа:
>>>>>>>> > Hi Galen,
>>>>>>>> >
>>>>>>>> > you've done a really great job in this patch!
>>>>>>>> >
>>>>>>>> > First let me apologize for making you wait so long, we were
>>>>>>>> really
>>>>>>>> busy
>>>>>>>> > lately. We have discussed the plugin off list already, but
it's
>>>>>>>> now
>>>>>>>> > applied and committed. You're also on our contributors page.
>>>>>>>> >
>>>>>>>> > (more inline)
>>>>>>>> >
>>>>>>>> > Damian Johnson wrote:
>>>>>>>> >> Hi. I've just finished integrating a chooser for keybindings
>>>>>>>> into
>>>>>>>> the SIP
>>>>>>>> >> Communicator. This both provides a means for users to change
>>>>>>>> their
>>>>>>>> >> keybinding preferences and save them between runs. Most of
the
>>>>>>>> details of
>>>>>>>> >> the addition are described in the latest blog post
>>>>>>>> >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
>>>>>>>> consists of
>>>>>>>> >> two additions:
>>>>>>>> >> 1. Keybinding persistence via a new service. This saves
custom
>>>>>>>> bindings with
>>>>>>>> >> the user's preferences and informs any frames using the
>>>>>>>> bindings
>>>>>>>> of updates.
>>>>>>>> >> *src/net/java/sip/communicator/impl/keybindings
>>>>>>>> >> src/net/java/sip/communicator/service/keybindings
>>>>>>>> >
>>>>>>>> > Applied. I like the way you have implemented key bindings in
the
>>>>>>>> gui bundle!
>>>>>>>> >
>>>>>>>> >> *
>>>>>>>> >> 2. Plugin that provides a ConfigurationForm for changing the
>>>>>>>> keybindings.
>>>>>>>> >> *src/net/java/sip/communicator/plugin/keybindingChooser*
>>>>>>>> >>
>>>>>>>> >> I've added a how-to for adding new keybinding sets
>>>>>>>> >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php
>(below
>>>>>>>> the
>>>>>>>> >> chooser description). Also, this is my first time making a
>>>>>>>> patch
>>>>>>>> >> - are binaries supposed to be provided separately like this?
I
>>>>>>>> didn't spot
>>>>>>>> >> an option to include them in svn diff…
>>>>>>>> >
>>>>>>>> > My reply is coming late, but yes binaries should be supplied
>>>>>>>> separately,
>>>>>>>> > because they're not contained in the diff. The how-to is also
>>>>>>>> very
>>>>>>>> good
>>>>>>>> > idea. Let me know if you're interested in adding this on the
SIP
>>>>>>>> > Communicator website.
>>>>>>>> >
>>>>>>>> >> A couple of usability features (any opinions?):
>>>>>>>> >> 1. Add a key to revert bindings to defaults. Helpful or
>>>>>>>> unnecessary?
>>>>>>>> >> 2. Keybindings aren't changed until the user hits "Apply".
>>>>>>>> Perhaps bindings
>>>>>>>> >> that haven't been applied should be discolored to draw
>>>>>>>> attention
>>>>>>>> to them?
>>>>>>>> >
>>>>>>>> > My personal opinion is that a button "Defaults" could be
really
>>>>>>>> helpful.
>>>>>>>> > Otherwise coloring unsaved bindings could be useful, but also
it
>>>>>>>> could
>>>>>>>> > be confusing, the user could not be sure what exactly the
color
>>>>>>>> means.
>>>>>>>> >
>>>>>>>> >> Known Issue:
>>>>>>>> >> Not all permissible bindings work in all situations. For
>>>>>>>> instance
>>>>>>>> in the
>>>>>>>> >> main frame the 'Contacts' and 'Chat rooms' panels appear to
>>>>>>>> intercept some
>>>>>>>> >> keyboard events, preventing bindings to directional arrows
>>>>>>>> (without
>>>>>>>> >> modifiers) or the space bar from working. This isn't a
terribly
>>>>>>>> big woop but
>>>>>>>> >> might possibly confuse users.
>>>>>>>> >
>>>>>>>> > Hm, could you explain more in details the problem situations,
we
>>>>>>>> could
>>>>>>>> > try to resolve this.
>>>>>>>> >
>>>>>>>> >> A few unrelated things I've come across:
>>>>>>>> >> 1. The patch also includes the fix for a minor (but very
>>>>>>>> consistent)
>>>>>>>> >> spelling error: 'desactivate' to 'deactivate'. This included
>>>>>>>> tweaking an
>>>>>>>> >> image with the word, changing some language files, and
>>>>>>>> refactoring parts of
>>>>>>>> >> the ManageButtonsPanel class.
>>>>>>>> >
>>>>>>>> > Committed all spelling corrections. Thanks.
>>>>>>>> >
>>>>>>>> >> 2. Does anyone know if there's a known issue with the
>>>>>>>> chatalerter
>>>>>>>> plugin?
>>>>>>>> >> Eclipse has complained that the org.jdesktop.jdic.misc
package
>>>>>>>> can't be
>>>>>>>> >> resolved ever since I checked it out via svn. However this
has
>>>>>>>> never
>>>>>>>> >> prevented the SIP Communicator from running. I've tried
asking
>>>>>>>> a
>>>>>>>> couple of
>>>>>>>> >> times on the IRC channel but I think I keep catching everyone
>>>>>>>> when they're
>>>>>>>> >> asleep…
>>>>>>>> >
>>>>>>>> > Maybe you've already figured it out yourself, but I'm just
>>>>>>>> replying for
>>>>>>>> > the record…Even that Eclipse is complaining about a library,
if
>>>>>>>> you
>>>>>>>> > compile and run SIP Communicator through Ant it will always
>>>>>>>> work,
>>>>>>>> > because the classpath is set inside the build.xml. If Eclipse
is
>>>>>>>> > complaining about a library this means that this library is
not
>>>>>>>> added in
>>>>>>>> > the .classpath file of Eclipse. You need to do the following:
>>>>>>>> right
>>>>>>>> > click on the project / "Properties" / "Project Build Path" /
>>>>>>>> "Libraries"
>>>>>>>> > / "Add new" / Find the library in the lib directory of SComm.
>>>>>>>> >
>>>>>>>> >> 3. Couple of minor issues on the site:
>>>>>>>> >> Developer Documentation > How to Configure Eclipse
>>>>>>>> >> Minor simplification- it's not necessary to get the old
version
>>>>>>>> of Subclipse
>>>>>>>> >> then upgrade it. The buckminster error can be avoided by
>>>>>>>> disabling the
>>>>>>>> >> optional components.
>>>>>>>> >
>>>>>>>> > Nice catch! I didn't experienced this error, so I'm not very
>>>>>>>> familiar
>>>>>>>> > with the problem, so again are you interested in changing the
>>>>>>>> part
>>>>>>>> of
>>>>>>>> > the tutorial concerning the problem.
>>>>>>>> >
>>>>>>>> >> Developer Documentation > UI Service > Create and add
>>>>>>>> configuration forms
>>>>>>>> >> As Atul noticed the 'getConfigurationManager()' method
doesn't
>>>>>>>> exist and
>>>>>>>> >> should be 'getConfigurationWindow()'.
>>>>>>>> >
>>>>>>>> > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
>>>>>>>> this one,
>>>>>>>> > could you be more precise?
>>>>>>>> >
>>>>>>>> > Damian, the patch was very well documented, very mature and
easy
>>>>>>>> to
>>>>>>>> > read. Again you've done a very good work! Thank you for the
>>>>>>>> patience and
>>>>>>>> > keep up the good work:)
>>>>>>>> >
>>>>>>>> > Cheers,
>>>>>>>> > Yana
>>>>>>>> >
>>>>>>>> >> Hope this helps. Cheers! -Galen
>>>>>>>> >>
>>>>>>>> >> PS. I'm planning to try going by my middle name, Galen, in
SIP
>>>>>>>> >> correspondence to avoid confusion with Damien and the other
>>>>>>>> Damian. :slight_smile:
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> >> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> >> For additional commands, e-mail:
>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> >
>>>>>>>
>>>>>>>> >
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> > To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> > For additional commands, e-mail:
>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> >
>>>>>>>
>>>>>>>> >
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> For additional commands, e-mail:
>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>>
>>>>>>>
------------------------------------------------------------------------
>>>>>>>>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> For additional commands, e-mail:
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>>
>>>>>>>>
---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>> For additional commands, e-mail:
>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>>>>>> For additional commands, e-mail:
>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>
>>>>>>
>>>>>>
>>>>>>
---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>>>>> For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
>>>>>
>>>>>
>>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
>>> For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
>>>
>>>
>>
>


#12

Hey Damian,

Thanks for implementing the changes! The screenshot looks very nice!

I've also had a quick look on the patch itself and it seemed fine to
me. Good work!

Unless someone else takes it though I won't be able to apply it before
next week so I might have a few questions when I get back to it.

Cheers
Emil

···

On Thu, Jul 31, 2008 at 10:00 AM, Damian Johnson <atagar1@gmail.com> wrote:

Hi. I've just finished implementing all the requested changes for the
keybindings service. In addition to the previous change list there's now
alternative binding support (see attached screenshot), the jar's been
unpacked (and removed) and the service no longer uses binaries for default
or custom bindings. The following example uses the bindings added for the
chat toolbar.

In defaults.properties the binding information looks like:
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.index=0
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcut=ctrl
pressed B
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcutAlt=ctrl
pressed F
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.index=1
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcut=ctrl
pressed I
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcutAlt=ctrl
pressed K
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.index=2
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.shortcut=ctrl
pressed U

Field's functions should be self explanatory and the alternate shortcuts
('shortcutAlt') are optional. Custom bindings are stored via the
ConfigurationService (in my case to
~/.sip-communicator/sip-communicator.xml) and look something like:
<keybindings>
  <chatToolbar>
    <bold value="ctrl pressed B"/>
    <bold-alt value="ctrl pressed F"/>
    <italic value="ctrl pressed I"/>
    <italic-alt value="ctrl pressed K"/>
    <underline value="ctrl pressed U"/>
    <underline-alt value=""/>
  </chatToolbar>
</keybindings>

Quite a bit was removed and I'm not sure if the patch reflects that so I've
included the svn status output to give an indication of where changes were
made. Unlike the normal bindings, the alternatives can be removed (with the
escape key). One other note is that I added an
'isSettingsStringAvailable(String)' method to the ResourceManagementService
since otherwise there's no means of figuring out if a default exists.
Personally I think the getSettingsString(String) method should throw the
MissingResourceException rather than log errors (and have a javadoc
description) but I'm not sure if changing this would upset other classes
that currently use it. Cheers! -Damian

2008/7/27 Emil Ivov <emcho@sip-communicator.org>

Hey Damian,

Glad to hear you are making progress on this one!

The defaults.properties file was indeed previewed for cases just like
yours: the conf service is supposed to load it as default content the
first time it runs.

However, as you have noticed this behaviour hasn't been fully
implemented yet so right now you can either implement it yourself or
do as Damien suggested - use a resource file inside your bundle and in
case your properties are not already in the ConfigurationService, load
them in there by your self.

Cheers
Emil

On 7/27/08, Damian Johnson <atagar1@gmail.com> wrote:
> Hi. I've tweaked the KeybindingsService implementation to utilize the
> ConfigurationService so the custom bindings are now stored in a
> human-editable fashion. However, I'm having some difficulty finding
> where
> the defaults should be stored. The obvious choice is
> 'resources/config/defaults.properties' but the
> ResourceManagementService,
> unlike ConfigurationService, isn't structured as a tree and hence
> doesn't
> have any methods for retrieving batches of defaults (ie, all chat
> keybindings). Per chance is there a file with the initial values of
> '~/.sip-communicator/sip-communicator.xml'? -Damian
>
> 2008/7/25 Damian Johnson <atagar1@gmail.com>
>
>> Hi Yana. Thanks for the quick response! Previous discussion on this
>> thread
>> concerned the binary property files. I chose serialized linked hash
>> maps
>> so
>> ordering would be preserved (which is important for the appearance).
>> Emil's
>> suggested adding an index to the plain text properties and I'm planning
>> to
>> look into this soon. -Damian
>>
>> 2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:
>>
>> Hi Damian,
>>>
>>> Damian Johnson wrote:
>>>
>>>> Hi. Thus far I've made the following changes:
>>>> Added 'Set Default' button
>>>> Added bindings for fonts
>>>> Merged keybindingchoosr plugin into keybindings service
>>>> implementation
>>>> Bug fix: Issue with the escape binding (thanks, Pavel, for catching
>>>> that!)
>>>> Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
>>>> NullPointerException if user cancels the color chooser
>>>> Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
>>>> 'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths
>>>>
>>>
>>> Good job!
>>>
>>>
>>>> However, I've come up against a bit of a chicken-and-egg problem. The
>>>> swing-ui bundle's activator uses the keybindings service to
>>>> initialize
>>>> the
>>>> UI, hence that service needs to start first. However, the keybinding
>>>> chooser
>>>> needs the swing-ui to set up the custom look-and-feel (the
>>>> ConfigurationForm
>>>> is using the swing default now that it's starting first).
>>>>
>>>> I think we should merge the keybindings service into swing-ui (*
>>>> /impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
>>>> beyond
>>>> this issue:
>>>> 1. Currently the KeybindingsService is being provided through the
>>>> GuiActivator (part of the swing-ui bundle). Actually, the keybindings
>>>> service isn't available to plugins unless we're gonna make the
>>>> service
>>>> available through something like the UIService.
>>>> 2. Keybindings are strictly a UI feature anyway…
>>>>
>>>> Does this sound good? If not, any thoughts on alternatives? Cheers!
>>>> -Damian
>>>>
>>>
>>> Sounds very good. I have also discussed this with Emil off list and he
>>> also agrees, so you could go ahead and integrate it in the swing-ui.
>>>
>>> Just one more thing. I'm not sure if Emil has already asked, but is
>>> there
>>> any special reason for which the configuration files for keybindings :
>>> keybindings-chat and keybindings-main are binary files? Could we make
>>> them
>>> just properties files, so that they're easier to read?
>>>
>>> Cheers,
>>> Yana
>>>
>>>
>>>
>>>> 2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:
>>>>
>>>> Hi Pavel,
>>>>>
>>>>> I have committed a fix and the font button should be now working
>>>>> (from
>>>>> build 1242). Let me know if you continue experiencing problems.
>>>>>
>>>>> Cheers,
>>>>> Yana
>>>>>
>>>>>
>>>>> Yana Stamcheva wrote:
>>>>>
>>>>> Hi Pavel,
>>>>>>
>>>>>> thanks for reporting!
>>>>>>
>>>>>> I think that only the Escape key problem is related to Damian's
>>>>>> work,
>>>>>> but
>>>>>> I'm confirming the two other problems.
>>>>>>
>>>>>> I'll have a look at the font button asap.
>>>>>>
>>>>>> Is there someone else experiencing encoding problems when
>>>>>> sending/receiving messages ? The first thing that comes to me is
>>>>>> that
>>>>>> it
>>>>>> could be due to recent modifications for HTML message support.
>>>>>>
>>>>>> Cheers,
>>>>>> Yana
>>>>>>
>>>>>> Pavel Tankov wrote:
>>>>>>
>>>>>> Hello Damian,
>>>>>>>
>>>>>>> It looks like there are a few problems now with the new keybinding
>>>>>>> plugin. Here are a few I've noticed that worked before, but now
>>>>>>> don't:
>>>>>>> - Escape key doesn't close chat window;
>>>>>>> - The button for selecting font doesn't popup the selection
>>>>>>> window;
>>>>>>> - My messages sent in cyrillic appear as ??? on the other side.
>>>>>>> Probably that's true for other non-latin encodings.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Pavel Tankov
>>>>>>>
>>>>>>> ----- Original Message ----
>>>>>>> From: Damian Johnson <atagar1@gmail.com>
>>>>>>> To: dev@sip-communicator.dev.java.net
>>>>>>> Sent: Saturday, July 19, 2008 9:22:57 AM
>>>>>>> Subject: Re: [sip-comm-dev] Keybinding Chooser Addition
>>>>>>>
>>>>>>>
>>>>>>> Hi Emil,
>>>>>>>
>>>>>>> I am not sure I see why this prevents us from copying the content
>>>>>>> of
>>>>>>> the
>>>>>>> plugin.keybindingchooser package into impl.keybindings.
>>>>>>> Ack! Sorry, I misread your original comment and thought the
>>>>>>> proposition
>>>>>>> was to do away with the service. My bad.
>>>>>>> I was trying to say that since the configuration UI is quite
>>>>>>> tightly
>>>>>>> coupled with this service implementation and both are quite light
>>>>>>> then
>>>>>>> they
>>>>>>> probably should belong to the same package… It would have made
>>>>>>> sense to
>>>>>>> keep them separated if there's a chance of someone needing
>>>>>>> concurrent
>>>>>>> versions of the chooser UI, which I don't think is very probable.
>>>>>>> Good points- I'll make the change.
>>>>>>> Well unless I am missing something we'll probably only need
>>>>>>> what's
>>>>>>> in
>>>>>>> the chooser package which gives us approximately 1400 lines of
>>>>>>> code.
>>>>>>> The
>>>>>>> main reason I am insisting on this is that we'll want to modify
>>>>>>> the
>>>>>>> UI
>>>>>>> and
>>>>>>> having the code in there would make it easier… If you don't feel
>>>>>>> like
>>>>>>> making the change then, by all means, just leave it to us. Your
>>>>>>> contribution
>>>>>>> is appreciated as it is
>>>>>>> As I mentioned if we're gonna make substantial changes to the UI
>>>>>>> then
>>>>>>> this makes sense, and the 'alternative bindings' are certainly
>>>>>>> substantial
>>>>>>> enough. I'm sorry if I gave the impression that I wasn't willing
>>>>>>> to
>>>>>>> make the
>>>>>>> change- I'll handle it.
>>>>>>> … I meant alternative bindings
>>>>>>> I've never seen this feature before but should be simple once the
>>>>>>> chooser's source is added in.
>>>>>>> Hopefully we'll soon be able to start integrating the spell
>>>>>>> checker
>>>>>>> soon
>>>>>>> too. :slight_smile:
>>>>>>> Cheers,
>>>>>>> Damian
>>>>>>> PS. I give up on going by 'Galen'. With webmail metadata adding
>>>>>>> in
>>>>>>> my
>>>>>>> name this is just confusing. Oh well- it was worth a try…
>>>>>>> On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < >> >>>>>>> emcho@sip-communicator.org> >> >>>>>>> wrote:
>>>>>>>
>>>>>>> Hey Damian,
>>>>>>>
>>>>>>> Damian Johnson написа:
>>>>>>>
>>>>>>> First, I don't quite see the need of splitting the
>>>>>>> implementation
>>>>>>>
>>>>>>>> into a service and a plugin.
>>>>>>>>
>>>>>>>> Actually I was just thinking about this. Are we going to want
>>>>>>>> plugins
>>>>>>>> to
>>>>>>>> be able to listen for and process keyboard events?
>>>>>>>>
>>>>>>>> Most probably.
>>>>>>>
>>>>>>>
>>>>>>> If so then the
>>>>>>>
>>>>>>>> addition shouldn't use the new methods provided via the
>>>>>>>> SIPCommFrame.
>>>>>>>> If
>>>>>>>> not then you're completely right.
>>>>>>>>
>>>>>>>> I am not sure I see why this prevents us from copying the
>>>>>>>> content
>>>>>>>> of
>>>>>>> the
>>>>>>> plugin.keybindingchooser package into impl.keybindings.
>>>>>>>
>>>>>>> Guess I wasn't clear. Apologies. I was trying to say that since
>>>>>>> the
>>>>>>> configuration UI is quite tightly coupled with this service
>>>>>>> implementation and both are quite light then they probably should
>>>>>>> belong
>>>>>>> to the same package.
>>>>>>>
>>>>>>>
>>>>>>> The configuration UI could very well live in impl.keybinding.
>>>>>>>
>>>>>>>> Certainly could. Assuming developers aren't interested in making
>>>>>>>> alternative choosers (which I doubt) there's no need for it to
>>>>>>>> act
>>>>>>>> as
>>>>>>>> a
>>>>>>>> plugin.
>>>>>>>>
>>>>>>>> I agree, developers are likely to tackle the UI but since both
>>>>>>>> the
>>>>>>> plugin and the impl are quite light, merging them would not make
>>>>>>> modifying the UI any more difficult. Quite, on the contrary, code
>>>>>>> would
>>>>>>> be easier to read since it would all be in the same place.
>>>>>>>
>>>>>>> It would have made sense to keep them separated if there's a
>>>>>>> chance
>>>>>>> of
>>>>>>> someone needing concurrent versions of the chooser UI, which I
>>>>>>> don't
>>>>>>> think is very probable.
>>>>>>>
>>>>>>>
>>>>>>> It would also be nice if we could copy necessary classes from
>>>>>>> your
>>>>>>>
>>>>>>>> lib into the service implementation.
>>>>>>>>
>>>>>>>> It would be easy to do- autoformatting is a beautiful thing so
>>>>>>>> getting
>>>>>>>> it to conform to the coding guidelines would be a breeze.
>>>>>>>> However,
>>>>>>>> unless we're gonna make radical changes to the UI I don't think
>>>>>>>> this
>>>>>>>> is
>>>>>>>> a good idea. Here's a couple of issues:
>>>>>>>>
>>>>>>>> 1. As the how-to mentions the jar is executable, providing an
>>>>>>>> editor
>>>>>>>> to
>>>>>>>> easily tweak the keybindings. If integrated we'd still probably
>>>>>>>> need
>>>>>>>> the
>>>>>>>> jar for that functionality (with the exception of the Persistence
>>>>>>>> class
>>>>>>>> that package probably woulnd't be integrated).
>>>>>>>>
>>>>>>>> I was actually thinking that if we start saving everything in
>>>>>>>> text
>>>>>>> form
>>>>>>> then we won't need to use the standalone tool when generating or
>>>>>>> modifying the default settings.
>>>>>>>
>>>>>>>
>>>>>>> 2. As far as hacks go, what's used is pretty short and sweet
>>>>>>> (providing
>>>>>>>
>>>>>>>> an I18N label abstraction and stretching the layout a bit). I
>>>>>>>> doubt
>>>>>>>> breaking open the jar would clarify anything- it would certainly
>>>>>>>> give
>>>>>>>> a
>>>>>>>> lot more code to look at.
>>>>>>>>
>>>>>>>> Well unless I am missing something we'll probably only need
>>>>>>>> what's
>>>>>>> in
>>>>>>> the chooser package which gives us approximately 1400 lines of
>>>>>>> code.
>>>>>>> The
>>>>>>> main reason I am insisting on this is that we'll want to modify
>>>>>>> the
>>>>>>> UI
>>>>>>> and having the code in there would make it easier.
>>>>>>>
>>>>>>> If you don't feel like making the change then, by all means, just
>>>>>>> leave
>>>>>>> it to us. Your contribution is appreciated as it is.
>>>>>>>
>>>>>>>
>>>>>>> Then, storing default and user preferences for key bindings in
>>>>>>> a
>>>>>>>
>>>>>>>> binary form makes it difficult to change them for no real
>>>>>>>> reason.
>>>>>>>> I've just seen that your lib already allows to store settings
>>>>>>>> in a
>>>>>>>> properties file so I think this would fit better.
>>>>>>>>
>>>>>>>> I actually spent quite a while antagonizing over this point.
>>>>>>>>
>>>>>>>> The point is that we will probably need to modify defaults quite
>>>>>>> often
>>>>>>> and not being able to do so by editing a text file would be a
>>>>>>> problem.
>>>>>>>
>>>>>>>
>>>>>>> The
>>>>>>>
>>>>>>>> advantage of storing it as a serialized linked hash map is that
>>>>>>>> it
>>>>>>>> retains ordering information, which is important from a UI
>>>>>>>> perspective
>>>>>>>> (so options like 'next tab' and 'previous tab' are shown next to
>>>>>>>> each
>>>>>>>> other). If that wasn't a concern then certainly- viva la plain
>>>>>>>> text.
>>>>>>>>
>>>>>>>> You can easily implement this with our ConfigurationService by
>>>>>>> manually
>>>>>>> adding indexes. Here's a very rough example:
>>>>>>>
>>>>>>> …
>>>>>>> <impl>
>>>>>>> <keybindings>
>>>>>>> <binding1>
>>>>>>> <actionName value="Previous Tab"/>
>>>>>>> …
>>>>>>> </binding1>
>>>>>>> <binding2>
>>>>>>> <actionName value="Next Tab"/>
>>>>>>> …
>>>>>>> </binding2>
>>>>>>> </keybindings>
>>>>>>> </impl>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> We can put the default ones into the defaults.properties files
>>>>>>> and
>>>>>>>
>>>>>>>> then use the configuration service to modify them.
>>>>>>>>
>>>>>>>> It looks like this service has changed quite a bit recently. I'll
>>>>>>>> look
>>>>>>>> into it.
>>>>>>>>
>>>>>>>> Yes the defaults are new indeed.
>>>>>>>
>>>>>>>
>>>>>>> 1. Would it be possible to implement support for alternative
>>>>>>>
>>>>>>>> actions? Most key binding management UIs have it would be
>>>>>>>> really
>>>>>>>> nice if we also did.
>>>>>>>>
>>>>>>>> I'm not familiar with this. Alternative action of what?
>>>>>>>>
>>>>>>>> Sorry, my bad, I meant alternative bindings. (See attached PNG)
>>>>>>>
>>>>>>>
>>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>>>>>>> Ctrl-i,
>>>>>>>
>>>>>>>> Ctrl-u shortcuts that we've recently added to the chat window?
>>>>>>>>
>>>>>>>> Oops! That's what I get for failing to sync with the trunk.
>>>>>>>> Simple
>>>>>>>> once
>>>>>>>> I get to a beefier connection.
>>>>>>>>
>>>>>>>> Thanks, I was hoping you'd say so :wink:
>>>>>>>
>>>>>>>
>>>>>>> Thanks again for the feedback!
>>>>>>> Thank you Damian, I do like the feature very much!
>>>>>>>
>>>>>>> Cheers
>>>>>>> Emil
>>>>>>>
>>>>>>> Cheers,
>>>>>>>
>>>>>>>> Galen
>>>>>>>>
>>>>>>>> On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < >> >>>>>>>> emcho@sip-communicator.org >> >>>>>>>> >> >>>>>>>> <mailto:emcho@sip-communicator.org>> wrote:
>>>>>>>
>>>>>>>> Hey Galen,
>>>>>>>>
>>>>>>>> Nice work indeed and the feature is quite useful. I've just
>>>>>>>> tried
>>>>>>>> it
>>>>>>>> and
>>>>>>>> I like it a lot. Thanks for contributing!
>>>>>>>>
>>>>>>>> There are however a few things that make me feel uncomfortable
>>>>>>>> with
>>>>>>>> the
>>>>>>>> way this is currently integrated in SIP Communicator.
>>>>>>>>
>>>>>>>> First, I don't quite see the need of splitting the
>>>>>>>> implementation
>>>>>>>> into a
>>>>>>>> service and a plugin. The configuration UI could very well live
>>>>>>>> in
>>>>>>>> impl.keybinding. It would also be nice if we could copy
>>>>>>>> necessary
>>>>>>>> classes from your lib into the service implementation. There
>>>>>>>> are
>>>>>>>> two
>>>>>>>> reasons I believe this would a good idea:
>>>>>>>> - We'd probably need to make changes on some parts such
>>>>>>>> as
>>>>>>>> the
>>>>>>>> configuration UI and the persistence mechanisms (see below) so
>>>>>>>> that
>>>>>>>> they
>>>>>>>> would fit the SC conventions.
>>>>>>>> - It would make the code easier to understand.
>>>>>>>>
>>>>>>>> Then, storing default and user preferences for key bindings in
>>>>>>>> a
>>>>>>>> binary
>>>>>>>> form makes it difficult to change them for no real reason. I've
>>>>>>>> just
>>>>>>>> seen that your lib already allows to store settings in a
>>>>>>>> properties
>>>>>>>> file
>>>>>>>> so I think this would fit better. We can put the default ones
>>>>>>>> into
>>>>>>>> the
>>>>>>>> defaults.properties files and then use the configuration
>>>>>>>> service
>>>>>>>> to
>>>>>>>> modify them.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> Other than that I was wondering if I could make the following
>>>>>>>> feature
>>>>>>>> requests:
>>>>>>>>
>>>>>>>> 1.Would it be possible to implement support for alternative
>>>>>>>> actions?
>>>>>>>> Most key binding management UIs have it would be really nice if
>>>>>>>> we
>>>>>>>> also did.
>>>>>>>> and
>>>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>>>>>>>> Ctrl-i,
>>>>>>>> Ctrl-u shortcuts that we've recently added to the chat window?
>>>>>>>>
>>>>>>>> Once again, thanks for contributing your work!
>>>>>>>>
>>>>>>>> Emil
>>>>>>>>
>>>>>>>>
>>>>>>>> Yana Stamcheva написа:
>>>>>>>> > Hi Galen,
>>>>>>>> >
>>>>>>>> > you've done a really great job in this patch!
>>>>>>>> >
>>>>>>>> > First let me apologize for making you wait so long, we were
>>>>>>>> really
>>>>>>>> busy
>>>>>>>> > lately. We have discussed the plugin off list already, but
>>>>>>>> it's
>>>>>>>> now
>>>>>>>> > applied and committed. You're also on our contributors page.
>>>>>>>> >
>>>>>>>> > (more inline)
>>>>>>>> >
>>>>>>>> > Damian Johnson wrote:
>>>>>>>> >> Hi. I've just finished integrating a chooser for keybindings
>>>>>>>> into
>>>>>>>> the SIP
>>>>>>>> >> Communicator. This both provides a means for users to change
>>>>>>>> their
>>>>>>>> >> keybinding preferences and save them between runs. Most of
>>>>>>>> the
>>>>>>>> details of
>>>>>>>> >> the addition are described in the latest blog post
>>>>>>>> >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
>>>>>>>> consists of
>>>>>>>> >> two additions:
>>>>>>>> >> 1. Keybinding persistence via a new service. This saves
>>>>>>>> custom
>>>>>>>> bindings with
>>>>>>>> >> the user's preferences and informs any frames using the
>>>>>>>> bindings
>>>>>>>> of updates.
>>>>>>>> >> *src/net/java/sip/communicator/impl/keybindings
>>>>>>>> >> src/net/java/sip/communicator/service/keybindings
>>>>>>>> >
>>>>>>>> > Applied. I like the way you have implemented key bindings in
>>>>>>>> the
>>>>>>>> gui bundle!
>>>>>>>> >
>>>>>>>> >> *
>>>>>>>> >> 2. Plugin that provides a ConfigurationForm for changing the
>>>>>>>> keybindings.
>>>>>>>> >> *src/net/java/sip/communicator/plugin/keybindingChooser*
>>>>>>>> >>
>>>>>>>> >> I've added a how-to for adding new keybinding sets
>>>>>>>> >>
>>>>>>>> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
>>>>>>>> the
>>>>>>>> >> chooser description). Also, this is my first time making a
>>>>>>>> patch
>>>>>>>> >> - are binaries supposed to be provided separately like this?
>>>>>>>> I
>>>>>>>> didn't spot
>>>>>>>> >> an option to include them in svn diff…
>>>>>>>> >
>>>>>>>> > My reply is coming late, but yes binaries should be supplied
>>>>>>>> separately,
>>>>>>>> > because they're not contained in the diff. The how-to is also
>>>>>>>> very
>>>>>>>> good
>>>>>>>> > idea. Let me know if you're interested in adding this on the
>>>>>>>> SIP
>>>>>>>> > Communicator website.
>>>>>>>> >
>>>>>>>> >> A couple of usability features (any opinions?):
>>>>>>>> >> 1. Add a key to revert bindings to defaults. Helpful or
>>>>>>>> unnecessary?
>>>>>>>> >> 2. Keybindings aren't changed until the user hits "Apply".
>>>>>>>> Perhaps bindings
>>>>>>>> >> that haven't been applied should be discolored to draw
>>>>>>>> attention
>>>>>>>> to them?
>>>>>>>> >
>>>>>>>> > My personal opinion is that a button "Defaults" could be
>>>>>>>> really
>>>>>>>> helpful.
>>>>>>>> > Otherwise coloring unsaved bindings could be useful, but also
>>>>>>>> it
>>>>>>>> could
>>>>>>>> > be confusing, the user could not be sure what exactly the
>>>>>>>> color
>>>>>>>> means.
>>>>>>>> >
>>>>>>>> >> Known Issue:
>>>>>>>> >> Not all permissible bindings work in all situations. For
>>>>>>>> instance
>>>>>>>> in the
>>>>>>>> >> main frame the 'Contacts' and 'Chat rooms' panels appear to
>>>>>>>> intercept some
>>>>>>>> >> keyboard events, preventing bindings to directional arrows
>>>>>>>> (without
>>>>>>>> >> modifiers) or the space bar from working. This isn't a
>>>>>>>> terribly
>>>>>>>> big woop but
>>>>>>>> >> might possibly confuse users.
>>>>>>>> >
>>>>>>>> > Hm, could you explain more in details the problem situations,
>>>>>>>> we
>>>>>>>> could
>>>>>>>> > try to resolve this.
>>>>>>>> >
>>>>>>>> >> A few unrelated things I've come across:
>>>>>>>> >> 1. The patch also includes the fix for a minor (but very
>>>>>>>> consistent)
>>>>>>>> >> spelling error: 'desactivate' to 'deactivate'. This included
>>>>>>>> tweaking an
>>>>>>>> >> image with the word, changing some language files, and
>>>>>>>> refactoring parts of
>>>>>>>> >> the ManageButtonsPanel class.
>>>>>>>> >
>>>>>>>> > Committed all spelling corrections. Thanks.
>>>>>>>> >
>>>>>>>> >> 2. Does anyone know if there's a known issue with the
>>>>>>>> chatalerter
>>>>>>>> plugin?
>>>>>>>> >> Eclipse has complained that the org.jdesktop.jdic.misc
>>>>>>>> package
>>>>>>>> can't be
>>>>>>>> >> resolved ever since I checked it out via svn. However this
>>>>>>>> has
>>>>>>>> never
>>>>>>>> >> prevented the SIP Communicator from running. I've tried
>>>>>>>> asking
>>>>>>>> a
>>>>>>>> couple of
>>>>>>>> >> times on the IRC channel but I think I keep catching
>>>>>>>> everyone
>>>>>>>> when they're
>>>>>>>> >> asleep…
>>>>>>>> >
>>>>>>>> > Maybe you've already figured it out yourself, but I'm just
>>>>>>>> replying for
>>>>>>>> > the record…Even that Eclipse is complaining about a library,
>>>>>>>> if
>>>>>>>> you
>>>>>>>> > compile and run SIP Communicator through Ant it will always
>>>>>>>> work,
>>>>>>>> > because the classpath is set inside the build.xml. If Eclipse
>>>>>>>> is
>>>>>>>> > complaining about a library this means that this library is
>>>>>>>> not
>>>>>>>> added in
>>>>>>>> > the .classpath file of Eclipse. You need to do the following:
>>>>>>>> right
>>>>>>>> > click on the project / "Properties" / "Project Build Path" /
>>>>>>>> "Libraries"
>>>>>>>> > / "Add new" / Find the library in the lib directory of SComm.
>>>>>>>> >
>>>>>>>> >> 3. Couple of minor issues on the site:
>>>>>>>> >> Developer Documentation > How to Configure Eclipse
>>>>>>>> >> Minor simplification- it's not necessary to get the old
>>>>>>>> version
>>>>>>>> of Subclipse
>>>>>>>> >> then upgrade it. The buckminster error can be avoided by
>>>>>>>> disabling the
>>>>>>>> >> optional components.
>>>>>>>> >
>>>>>>>> > Nice catch! I didn't experienced this error, so I'm not very
>>>>>>>> familiar
>>>>>>>> > with the problem, so again are you interested in changing the
>>>>>>>> part
>>>>>>>> of
>>>>>>>> > the tutorial concerning the problem.
>>>>>>>> >
>>>>>>>> >> Developer Documentation > UI Service > Create and add
>>>>>>>> configuration forms
>>>>>>>> >> As Atul noticed the 'getConfigurationManager()' method
>>>>>>>> doesn't
>>>>>>>> exist and
>>>>>>>> >> should be 'getConfigurationWindow()'.
>>>>>>>> >
>>>>>>>> > May be it's too late and I'm almost asleep :slight_smile: but I didn't
>>>>>>>> get
>>>>>>>> this one,
>>>>>>>> > could you be more precise?
>>>>>>>> >
>>>>>>>> > Damian, the patch was very well documented, very mature and
>>>>>>>> easy
>>>>>>>> to
>>>>>>>> > read. Again you've done a very good work! Thank you for the
>>>>>>>> patience and
>>>>>>>> > keep up the good work:)
>>>>>>>> >
>>>>>>>> > Cheers,
>>>>>>>> > Yana
>>>>>>>> >
>>>>>>>> >> Hope this helps. Cheers! -Galen
>>>>>>>> >>
>>>>>>>> >> PS. I'm planning to try going by my middle name, Galen, in
>>>>>>>> SIP
>>>>>>>> >> correspondence to avoid confusion with Damien and the other
>>>>>>>> Damian. :slight_smile:
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------------------------------------------------
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>>> >> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> >> For additional commands, e-mail:
>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> >
>>>>>>>
>>>>>>>> >
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>>> > To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> > For additional commands, e-mail:
>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> >
>>>>>>>
>>>>>>>> >
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>> For additional commands, e-mail:
>>>>>>>
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> ------------------------------------------------------------------------
>>>>>>>>
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail:
>>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>>> For additional commands, e-mail:
>>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>>
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail:
>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>>> For additional commands, e-mail:
>>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail:
>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>>> For additional commands, e-mail:
>>>>>> dev-help@sip-communicator.dev.java.net
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail:
>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>>>>> For additional commands, e-mail:
>>>>> dev-help@sip-communicator.dev.java.net
>>>>>
>>>>>
>>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
>>> For additional commands, e-mail:
>>> dev-help@sip-communicator.dev.java.net
>>>
>>>
>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#13

Hi Damian,

The screenshot is very nice!

However, in order to be able to commit your patch we would need you to change the license of your classes to the SIP Communicator license, instead of the Apache v2.

Otherwise I have also some questions:

- In the service/gui/gui.manifest.mf you have imported the javax.swing package. I didn't actually see where you use it inside the service, but we're avoiding putting in the service the swing package, because this would limit us to have only swing implementations of the service.

- I think we don't ever need the isSettingsStringAvailable(String key) method in the ResourceManagementService, since we're now returning null if such string does not exist, so this should be sufficient to check if it's available.

- What's the index property about in the configuration properties file? For example you have net.java.sip.communicator.impl.gui.keybindings.chat.nextTab.index=0.

- Just some more thoughts.. We should think of a method that would allow other bundles to register their keybindings in the gui. Something like:

registerKeybinding(Category, KeyStroke, String);

and also we should trigger events when something has changed:

addKeybindingListener(KeybindingListener l);

We could then use these instead of the Observer and the resetBindings method.

Cheers,
Yana

Damian Johnson wrote:

···

Hi. I've just finished implementing all the requested changes for the
keybindings service. In addition to the previous change list there's now
alternative binding support (see attached screenshot), the jar's been
unpacked (and removed) and the service no longer uses binaries for default
or custom bindings. The following example uses the bindings added for the
chat toolbar.

In defaults.properties the binding information looks like:
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.index=0
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcut=ctrl
pressed B
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcutAlt=ctrl
pressed F
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.index=1
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcut=ctrl
pressed I
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcutAlt=ctrl
pressed K
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.index=2
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.shortcut=ctrl
pressed U

Field's functions should be self explanatory and the alternate shortcuts
('shortcutAlt') are optional. Custom bindings are stored via the
ConfigurationService (in my case to
~/.sip-communicator/sip-communicator.xml) and look something like:
<keybindings>
  <chatToolbar>
    <bold value="ctrl pressed B"/>
    <bold-alt value="ctrl pressed F"/>
    <italic value="ctrl pressed I"/>
    <italic-alt value="ctrl pressed K"/>
    <underline value="ctrl pressed U"/>
    <underline-alt value=""/>
  </chatToolbar>
</keybindings>

Quite a bit was removed and I'm not sure if the patch reflects that so I've
included the svn status output to give an indication of where changes were
made. Unlike the normal bindings, the alternatives can be removed (with the
escape key). One other note is that I added an
'isSettingsStringAvailable(String)' method to the ResourceManagementService
since otherwise there's no means of figuring out if a default exists.
Personally I think the getSettingsString(String) method should throw the
MissingResourceException rather than log errors (and have a javadoc
description) but I'm not sure if changing this would upset other classes
that currently use it. Cheers! -Damian

2008/7/27 Emil Ivov <emcho@sip-communicator.org>

Hey Damian,

Glad to hear you are making progress on this one!

The defaults.properties file was indeed previewed for cases just like
yours: the conf service is supposed to load it as default content the
first time it runs.

However, as you have noticed this behaviour hasn't been fully
implemented yet so right now you can either implement it yourself or
do as Damien suggested - use a resource file inside your bundle and in
case your properties are not already in the ConfigurationService, load
them in there by your self.

Cheers
Emil

On 7/27/08, Damian Johnson <atagar1@gmail.com> wrote:

Hi. I've tweaked the KeybindingsService implementation to utilize the
ConfigurationService so the custom bindings are now stored in a
human-editable fashion. However, I'm having some difficulty finding where
the defaults should be stored. The obvious choice is
'resources/config/defaults.properties' but the ResourceManagementService,
unlike ConfigurationService, isn't structured as a tree and hence doesn't
have any methods for retrieving batches of defaults (ie, all chat
keybindings). Per chance is there a file with the initial values of
'~/.sip-communicator/sip-communicator.xml'? -Damian

2008/7/25 Damian Johnson <atagar1@gmail.com>

Hi Yana. Thanks for the quick response! Previous discussion on this

thread

concerned the binary property files. I chose serialized linked hash maps
so
ordering would be preserved (which is important for the appearance).
Emil's
suggested adding an index to the plain text properties and I'm planning

to

look into this soon. -Damian

2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:

Hi Damian,

Damian Johnson wrote:

Hi. Thus far I've made the following changes:
Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching
that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

Good job!

However, I've come up against a bit of a chicken-and-egg problem. The
swing-ui bundle's activator uses the keybindings service to initialize
the
UI, hence that service needs to start first. However, the keybinding
chooser
needs the swing-ui to set up the custom look-and-feel (the
ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers!
-Damian

Sounds very good. I have also discussed this with Emil off list and he
also agrees, so you could go ahead and integrate it in the swing-ui.

Just one more thing. I'm not sure if Emil has already asked, but is

there

any special reason for which the configuration files for keybindings :
keybindings-chat and keybindings-main are binary files? Could we make
them
just properties files, so that they're easier to read?

Cheers,
Yana

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working

(from

build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's

work,

but
I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is

that

it
could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now
don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content

of

the
plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the
proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite

tightly

coupled with this service implementation and both are quite light
then
they
probably should belong to the same package.... It would have made
sense to
keep them separated if there's a chance of someone needing

concurrent

versions of the chooser UI, which I don't think is very probable.
Good points- I'll make the change.
Well unless I am missing something we'll probably only need what's
in
the chooser package which gives us approximately 1400 lines of

code.

The
main reason I am insisting on this is that we'll want to modify the
UI
and
having the code in there would make it easier... If you don't feel
like
making the change then, by all means, just leave it to us. Your
contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI

then

this makes sense, and the 'alternative bindings' are certainly
substantial
enough. I'm sorry if I gave the impression that I wasn't willing to
make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell

checker

soon
too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding in
my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < >>>>>>>>> emcho@sip-communicator.org> >>>>>>>>> wrote:

Hey Damian,

Damian Johnson ������:

   First, I don't quite see the need of splitting the

implementation

  into a service and a plugin.

Actually I was just thinking about this. Are we going to want
plugins
to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the
SIPCommFrame.
If
not then you're completely right.

I am not sure I see why this prevents us from copying the content
of

the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should
belong
to the same package.

   The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act
as
a
plugin.

I agree, developers are likely to tackle the UI but since both

the

plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code
would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance
of
someone needing concurrent versions of the chooser UI, which I

don't

think is very probable.

   It would also be nice if we could copy necessary classes from

your

  lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so
getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think

this

is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an

editor

to
easily tweak the keybindings. If integrated we'd still probably

need

the
jar for that functionality (with the exception of the Persistence
class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in

text

form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet
(providing

an I18N label abstraction and stretching the layout a bit). I

doubt

breaking open the jar would clarify anything- it would certainly
give
a
lot more code to look at.

Well unless I am missing something we'll probably only need

what's

in
the chooser package which gives us approximately 1400 lines of

code.

The
main reason I am insisting on this is that we'll want to modify the
UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just
leave
it to us. Your contribution is appreciated as it is.

   Then, storing default and user preferences for key bindings in a

  binary form makes it difficult to change them for no real

reason.

  I've just seen that your lib already allows to store settings in

a

  properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite

often
and not being able to do so by editing a text file would be a
problem.

The

advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI
perspective
(so options like 'next tab' and 'previous tab' are shown next to
each
other). If that wasn't a concern then certainly- viva la plain

text.

You can easily implement this with our ConfigurationService by

manually
adding indexes. Here's a very rough example:

....
<impl>
<keybindings>
     <binding1>
         <actionName value="Previous Tab"/>
         ...
     </binding1>
     <binding2>
         <actionName value="Next Tab"/>
         ...
     </binding2>
</keybindings>
</impl>

   We can put the default ones into the defaults.properties files

and

  then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll
look
into it.

Yes the defaults are new indeed.

   1. Would it be possible to implement support for alternative

  actions? Most key binding management UIs have it would be really
  nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

   2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,

  Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple
once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!
Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < >>>>>>>>>> emcho@sip-communicator.org >>>>>>>>>> >>>>>>>>>> <mailto:emcho@sip-communicator.org>> wrote:
  Hey Galen,

  Nice work indeed and the feature is quite useful. I've just

tried

it
and
  I like it a lot. Thanks for contributing!

  There are however a few things that make me feel uncomfortable
with
the
  way this is currently integrated in SIP Communicator.

  First, I don't quite see the need of splitting the

implementation

into a
  service and a plugin. The configuration UI could very well live

in

  impl.keybinding. It would also be nice if we could copy

necessary

  classes from your lib into the service implementation. There are
two
  reasons I believe this would a good idea:
         - We'd probably need to make changes on some parts such

as

the
  configuration UI and the persistence mechanisms (see below) so
that
they
  would fit the SC conventions.
         - It would make the code easier to understand.

  Then, storing default and user preferences for key bindings in a
binary
  form makes it difficult to change them for no real reason. I've
just
  seen that your lib already allows to store settings in a
properties
file
  so I think this would fit better. We can put the default ones

into

the
  defaults.properties files and then use the configuration service
to
  modify them.

  What do you think?

  Other than that I was wondering if I could make the following
feature
  requests:

  1.Would it be possible to implement support for alternative
actions?
  Most key binding management UIs have it would be really nice if

we

  also did.
  and
  2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,
  Ctrl-u shortcuts that we've recently added to the chat window?

  Once again, thanks for contributing your work!

  Emil

  Yana Stamcheva ������:
  > Hi Galen,
  >
  > you've done a really great job in this patch!
  >
  > First let me apologize for making you wait so long, we were
really
  busy
  > lately. We have discussed the plugin off list already, but

it's

now
  > applied and committed. You're also on our contributors page.
  >
  > (more inline)
  >
  > Damian Johnson wrote:
  >> Hi. I've just finished integrating a chooser for keybindings
into
  the SIP
  >> Communicator. This both provides a means for users to change
their
  >> keybinding preferences and save them between runs. Most of

the

  details of
  >> the addition are described in the latest blog post
  >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
  >> two additions:
  >> 1. Keybinding persistence via a new service. This saves

custom

  bindings with
  >> the user's preferences and informs any frames using the
bindings
  of updates.
  >> *src/net/java/sip/communicator/impl/keybindings
  >> src/net/java/sip/communicator/service/keybindings
  >
  > Applied. I like the way you have implemented key bindings in

the

  gui bundle!
  >
  >> *
  >> 2. Plugin that provides a ConfigurationForm for changing the
  keybindings.
  >> *src/net/java/sip/communicator/plugin/keybindingChooser*
  >>
  >> I've added a how-to for adding new keybinding sets
  >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php

(below

the
  >> chooser description). Also, this is my first time making a
patch
  >> - are binaries supposed to be provided separately like this?

I

  didn't spot
  >> an option to include them in svn diff...
  >
  > My reply is coming late, but yes binaries should be supplied
  separately,
  > because they're not contained in the diff. The how-to is also
very
  good
  > idea. Let me know if you're interested in adding this on the

SIP

  > Communicator website.
  >
  >> A couple of usability features (any opinions?):
  >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
  >> 2. Keybindings aren't changed until the user hits "Apply".
  Perhaps bindings
  >> that haven't been applied should be discolored to draw
attention
  to them?
  >
  > My personal opinion is that a button "Defaults" could be

really

  helpful.
  > Otherwise coloring unsaved bindings could be useful, but also

it

could
  > be confusing, the user could not be sure what exactly the

color

means.
  >
  >> Known Issue:
  >> Not all permissible bindings work in all situations. For
instance
  in the
  >> main frame the 'Contacts' and 'Chat rooms' panels appear to
  intercept some
  >> keyboard events, preventing bindings to directional arrows
(without
  >> modifiers) or the space bar from working. This isn't a

terribly

  big woop but
  >> might possibly confuse users.
  >
  > Hm, could you explain more in details the problem situations,

we

could
  > try to resolve this.
  >
  >> A few unrelated things I've come across:
  >> 1. The patch also includes the fix for a minor (but very
consistent)
  >> spelling error: 'desactivate' to 'deactivate'. This included
  tweaking an
  >> image with the word, changing some language files, and
  refactoring parts of
  >> the ManageButtonsPanel class.
  >
  > Committed all spelling corrections. Thanks.
  >
  >> 2. Does anyone know if there's a known issue with the
chatalerter
  plugin?
  >> Eclipse has complained that the org.jdesktop.jdic.misc

package

  can't be
  >> resolved ever since I checked it out via svn. However this

has

never
  >> prevented the SIP Communicator from running. I've tried

asking

a
  couple of
  >> times on the IRC channel but I think I keep catching everyone
  when they're
  >> asleep...
  >
  > Maybe you've already figured it out yourself, but I'm just
  replying for
  > the record..Even that Eclipse is complaining about a library,

if

you
  > compile and run SIP Communicator through Ant it will always
work,
  > because the classpath is set inside the build.xml. If Eclipse

is

  > complaining about a library this means that this library is

not

  added in
  > the .classpath file of Eclipse. You need to do the following:
right
  > click on the project / "Properties" / "Project Build Path" /
  "Libraries"
  > / "Add new" / Find the library in the lib directory of SComm.
  >
  >> 3. Couple of minor issues on the site:
  >> Developer Documentation > How to Configure Eclipse
  >> Minor simplification- it's not necessary to get the old

version

  of Subclipse
  >> then upgrade it. The buckminster error can be avoided by
  disabling the
  >> optional components.
  >
  > Nice catch! I didn't experienced this error, so I'm not very
familiar
  > with the problem, so again are you interested in changing the
part
of
  > the tutorial concerning the problem.
  >
  >> Developer Documentation > UI Service > Create and add
  configuration forms
  >> As Atul noticed the 'getConfigurationManager()' method

doesn't

  exist and
  >> should be 'getConfigurationWindow()'.
  >
  > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
  this one,
  > could you be more precise?
  >
  > Damian, the patch was very well documented, very mature and

easy

to
  > read. Again you've done a very good work! Thank you for the
  patience and
  > keep up the good work:)
  >
  > Cheers,
  > Yana
  >
  >> Hope this helps. Cheers! -Galen
  >>
  >> PS. I'm planning to try going by my middle name, Galen, in

SIP

  >> correspondence to avoid confusion with Damien and the other
  Damian. :slight_smile:
  >>

------------------------------------------------------------------------

  >>

------------------------------------------------------------------------

  >>

------------------------------------------------------------------------

  >>

---------------------------------------------------------------------

  >> To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    >> For additional commands, e-mail:
  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >
  >

---------------------------------------------------------------------

  > To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    > For additional commands, e-mail:
  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

    >
  >

---------------------------------------------------------------------

  To unsubscribe, e-mail:
  dev-unsubscribe@sip-communicator.dev.java.net
  <mailto:dev-unsubscribe@sip-communicator.dev.java.net>

    For additional commands, e-mail:
  dev-help@sip-communicator.dev.java.net
  <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------

To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

------------------------------------------------------------------------

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#14

Hi. I've just made a minor edit so the KeybindingService is available to
plugins via the UIService. I also added documentation for how to add
keybindings here<http://www.sip-communicator.org/index.php/Documentation/HowToAddKeyBindings>.
Cheers! -Damian

keybindingsUpdate2.patch (178 KB)

···

On Thu, Jul 31, 2008 at 10:08 AM, Emil Ivov <emcho@sip-communicator.org>wrote:

Hey Damian,

Thanks for implementing the changes! The screenshot looks very nice!

I've also had a quick look on the patch itself and it seemed fine to
me. Good work!

Unless someone else takes it though I won't be able to apply it before
next week so I might have a few questions when I get back to it.

Cheers
Emil

On Thu, Jul 31, 2008 at 10:00 AM, Damian Johnson <atagar1@gmail.com> > wrote:
> Hi. I've just finished implementing all the requested changes for the
> keybindings service. In addition to the previous change list there's now
> alternative binding support (see attached screenshot), the jar's been
> unpacked (and removed) and the service no longer uses binaries for
default
> or custom bindings. The following example uses the bindings added for the
> chat toolbar.
>
> In defaults.properties the binding information looks like:
> net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.index=0
>
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcut=ctrl
> pressed B
>
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcutAlt=ctrl
> pressed F
> net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.index=1
>
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcut=ctrl
> pressed I
>
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcutAlt=ctrl
> pressed K
>
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.index=2
>
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.shortcut=ctrl
> pressed U
>
> Field's functions should be self explanatory and the alternate shortcuts
> ('shortcutAlt') are optional. Custom bindings are stored via the
> ConfigurationService (in my case to
> ~/.sip-communicator/sip-communicator.xml) and look something like:
> <keybindings>
> <chatToolbar>
> <bold value="ctrl pressed B"/>
> <bold-alt value="ctrl pressed F"/>
> <italic value="ctrl pressed I"/>
> <italic-alt value="ctrl pressed K"/>
> <underline value="ctrl pressed U"/>
> <underline-alt value=""/>
> </chatToolbar>
> </keybindings>
>
> Quite a bit was removed and I'm not sure if the patch reflects that so
I've
> included the svn status output to give an indication of where changes
were
> made. Unlike the normal bindings, the alternatives can be removed (with
the
> escape key). One other note is that I added an
> 'isSettingsStringAvailable(String)' method to the
ResourceManagementService
> since otherwise there's no means of figuring out if a default exists.
> Personally I think the getSettingsString(String) method should throw the
> MissingResourceException rather than log errors (and have a javadoc
> description) but I'm not sure if changing this would upset other classes
> that currently use it. Cheers! -Damian
>
> 2008/7/27 Emil Ivov <emcho@sip-communicator.org>
>>
>> Hey Damian,
>>
>> Glad to hear you are making progress on this one!
>>
>> The defaults.properties file was indeed previewed for cases just like
>> yours: the conf service is supposed to load it as default content the
>> first time it runs.
>>
>> However, as you have noticed this behaviour hasn't been fully
>> implemented yet so right now you can either implement it yourself or
>> do as Damien suggested - use a resource file inside your bundle and in
>> case your properties are not already in the ConfigurationService, load
>> them in there by your self.
>>
>> Cheers
>> Emil
>>
>> On 7/27/08, Damian Johnson <atagar1@gmail.com> wrote:
>> > Hi. I've tweaked the KeybindingsService implementation to utilize the
>> > ConfigurationService so the custom bindings are now stored in a
>> > human-editable fashion. However, I'm having some difficulty finding
>> > where
>> > the defaults should be stored. The obvious choice is
>> > 'resources/config/defaults.properties' but the
>> > ResourceManagementService,
>> > unlike ConfigurationService, isn't structured as a tree and hence
>> > doesn't
>> > have any methods for retrieving batches of defaults (ie, all chat
>> > keybindings). Per chance is there a file with the initial values of
>> > '~/.sip-communicator/sip-communicator.xml'? -Damian
>> >
>> > 2008/7/25 Damian Johnson <atagar1@gmail.com>
>> >
>> >> Hi Yana. Thanks for the quick response! Previous discussion on this
>> >> thread
>> >> concerned the binary property files. I chose serialized linked hash
>> >> maps
>> >> so
>> >> ordering would be preserved (which is important for the appearance).
>> >> Emil's
>> >> suggested adding an index to the plain text properties and I'm
planning
>> >> to
>> >> look into this soon. -Damian
>> >>
>> >> 2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:
>> >>
>> >> Hi Damian,
>> >>>
>> >>> Damian Johnson wrote:
>> >>>
>> >>>> Hi. Thus far I've made the following changes:
>> >>>> Added 'Set Default' button
>> >>>> Added bindings for fonts
>> >>>> Merged keybindingchoosr plugin into keybindings service
>> >>>> implementation
>> >>>> Bug fix: Issue with the escape binding (thanks, Pavel, for catching
>> >>>> that!)
>> >>>> Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
>> >>>> NullPointerException if user cancels the color chooser
>> >>>> Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
>> >>>> 'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths
>> >>>>
>> >>>
>> >>> Good job!
>> >>>
>> >>>
>> >>>> However, I've come up against a bit of a chicken-and-egg problem.
The
>> >>>> swing-ui bundle's activator uses the keybindings service to
>> >>>> initialize
>> >>>> the
>> >>>> UI, hence that service needs to start first. However, the
keybinding
>> >>>> chooser
>> >>>> needs the swing-ui to set up the custom look-and-feel (the
>> >>>> ConfigurationForm
>> >>>> is using the swing default now that it's starting first).
>> >>>>
>> >>>> I think we should merge the keybindings service into swing-ui (*
>> >>>> /impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
>> >>>> beyond
>> >>>> this issue:
>> >>>> 1. Currently the KeybindingsService is being provided through the
>> >>>> GuiActivator (part of the swing-ui bundle). Actually, the
keybindings
>> >>>> service isn't available to plugins unless we're gonna make the
>> >>>> service
>> >>>> available through something like the UIService.
>> >>>> 2. Keybindings are strictly a UI feature anyway…
>> >>>>
>> >>>> Does this sound good? If not, any thoughts on alternatives? Cheers!
>> >>>> -Damian
>> >>>>
>> >>>
>> >>> Sounds very good. I have also discussed this with Emil off list and
he
>> >>> also agrees, so you could go ahead and integrate it in the swing-ui.
>> >>>
>> >>> Just one more thing. I'm not sure if Emil has already asked, but is
>> >>> there
>> >>> any special reason for which the configuration files for keybindings
:
>> >>> keybindings-chat and keybindings-main are binary files? Could we
make
>> >>> them
>> >>> just properties files, so that they're easier to read?
>> >>>
>> >>> Cheers,
>> >>> Yana
>> >>>
>> >>>
>> >>>
>> >>>> 2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:
>> >>>>
>> >>>> Hi Pavel,
>> >>>>>
>> >>>>> I have committed a fix and the font button should be now working
>> >>>>> (from
>> >>>>> build 1242). Let me know if you continue experiencing problems.
>> >>>>>
>> >>>>> Cheers,
>> >>>>> Yana
>> >>>>>
>> >>>>>
>> >>>>> Yana Stamcheva wrote:
>> >>>>>
>> >>>>> Hi Pavel,
>> >>>>>>
>> >>>>>> thanks for reporting!
>> >>>>>>
>> >>>>>> I think that only the Escape key problem is related to Damian's
>> >>>>>> work,
>> >>>>>> but
>> >>>>>> I'm confirming the two other problems.
>> >>>>>>
>> >>>>>> I'll have a look at the font button asap.
>> >>>>>>
>> >>>>>> Is there someone else experiencing encoding problems when
>> >>>>>> sending/receiving messages ? The first thing that comes to me is
>> >>>>>> that
>> >>>>>> it
>> >>>>>> could be due to recent modifications for HTML message support.
>> >>>>>>
>> >>>>>> Cheers,
>> >>>>>> Yana
>> >>>>>>
>> >>>>>> Pavel Tankov wrote:
>> >>>>>>
>> >>>>>> Hello Damian,
>> >>>>>>>
>> >>>>>>> It looks like there are a few problems now with the new
keybinding
>> >>>>>>> plugin. Here are a few I've noticed that worked before, but now
>> >>>>>>> don't:
>> >>>>>>> - Escape key doesn't close chat window;
>> >>>>>>> - The button for selecting font doesn't popup the selection
>> >>>>>>> window;
>> >>>>>>> - My messages sent in cyrillic appear as ??? on the other
side.
>> >>>>>>> Probably that's true for other non-latin encodings.
>> >>>>>>>
>> >>>>>>> Thanks,
>> >>>>>>> Pavel Tankov
>> >>>>>>>
>> >>>>>>> ----- Original Message ----
>> >>>>>>> From: Damian Johnson <atagar1@gmail.com>
>> >>>>>>> To: dev@sip-communicator.dev.java.net
>> >>>>>>> Sent: Saturday, July 19, 2008 9:22:57 AM
>> >>>>>>> Subject: Re: [sip-comm-dev] Keybinding Chooser Addition
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> Hi Emil,
>> >>>>>>>
>> >>>>>>> I am not sure I see why this prevents us from copying the
content
>> >>>>>>> of
>> >>>>>>> the
>> >>>>>>> plugin.keybindingchooser package into impl.keybindings.
>> >>>>>>> Ack! Sorry, I misread your original comment and thought the
>> >>>>>>> proposition
>> >>>>>>> was to do away with the service. My bad.
>> >>>>>>> I was trying to say that since the configuration UI is quite
>> >>>>>>> tightly
>> >>>>>>> coupled with this service implementation and both are quite
light
>> >>>>>>> then
>> >>>>>>> they
>> >>>>>>> probably should belong to the same package… It would have
made
>> >>>>>>> sense to
>> >>>>>>> keep them separated if there's a chance of someone needing
>> >>>>>>> concurrent
>> >>>>>>> versions of the chooser UI, which I don't think is very
probable.
>> >>>>>>> Good points- I'll make the change.
>> >>>>>>> Well unless I am missing something we'll probably only need
>> >>>>>>> what's
>> >>>>>>> in
>> >>>>>>> the chooser package which gives us approximately 1400 lines of
>> >>>>>>> code.
>> >>>>>>> The
>> >>>>>>> main reason I am insisting on this is that we'll want to modify
>> >>>>>>> the
>> >>>>>>> UI
>> >>>>>>> and
>> >>>>>>> having the code in there would make it easier… If you don't
feel
>> >>>>>>> like
>> >>>>>>> making the change then, by all means, just leave it to us. Your
>> >>>>>>> contribution
>> >>>>>>> is appreciated as it is
>> >>>>>>> As I mentioned if we're gonna make substantial changes to the UI
>> >>>>>>> then
>> >>>>>>> this makes sense, and the 'alternative bindings' are certainly
>> >>>>>>> substantial
>> >>>>>>> enough. I'm sorry if I gave the impression that I wasn't willing
>> >>>>>>> to
>> >>>>>>> make the
>> >>>>>>> change- I'll handle it.
>> >>>>>>> … I meant alternative bindings
>> >>>>>>> I've never seen this feature before but should be simple once
the
>> >>>>>>> chooser's source is added in.
>> >>>>>>> Hopefully we'll soon be able to start integrating the spell
>> >>>>>>> checker
>> >>>>>>> soon
>> >>>>>>> too. :slight_smile:
>> >>>>>>> Cheers,
>> >>>>>>> Damian
>> >>>>>>> PS. I give up on going by 'Galen'. With webmail metadata adding
>> >>>>>>> in
>> >>>>>>> my
>> >>>>>>> name this is just confusing. Oh well- it was worth a try…
>> >>>>>>> On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < > >> >>>>>>> emcho@sip-communicator.org> > >> >>>>>>> wrote:
>> >>>>>>>
>> >>>>>>> Hey Damian,
>> >>>>>>>
>> >>>>>>> Damian Johnson написа:
>> >>>>>>>
>> >>>>>>> First, I don't quite see the need of splitting the
>> >>>>>>> implementation
>> >>>>>>>
>> >>>>>>>> into a service and a plugin.
>> >>>>>>>>
>> >>>>>>>> Actually I was just thinking about this. Are we going to want
>> >>>>>>>> plugins
>> >>>>>>>> to
>> >>>>>>>> be able to listen for and process keyboard events?
>> >>>>>>>>
>> >>>>>>>> Most probably.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> If so then the
>> >>>>>>>
>> >>>>>>>> addition shouldn't use the new methods provided via the
>> >>>>>>>> SIPCommFrame.
>> >>>>>>>> If
>> >>>>>>>> not then you're completely right.
>> >>>>>>>>
>> >>>>>>>> I am not sure I see why this prevents us from copying the
>> >>>>>>>> content
>> >>>>>>>> of
>> >>>>>>> the
>> >>>>>>> plugin.keybindingchooser package into impl.keybindings.
>> >>>>>>>
>> >>>>>>> Guess I wasn't clear. Apologies. I was trying to say that since
>> >>>>>>> the
>> >>>>>>> configuration UI is quite tightly coupled with this service
>> >>>>>>> implementation and both are quite light then they probably
should
>> >>>>>>> belong
>> >>>>>>> to the same package.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> The configuration UI could very well live in impl.keybinding.
>> >>>>>>>
>> >>>>>>>> Certainly could. Assuming developers aren't interested in
making
>> >>>>>>>> alternative choosers (which I doubt) there's no need for it to
>> >>>>>>>> act
>> >>>>>>>> as
>> >>>>>>>> a
>> >>>>>>>> plugin.
>> >>>>>>>>
>> >>>>>>>> I agree, developers are likely to tackle the UI but since both
>> >>>>>>>> the
>> >>>>>>> plugin and the impl are quite light, merging them would not make
>> >>>>>>> modifying the UI any more difficult. Quite, on the contrary,
code
>> >>>>>>> would
>> >>>>>>> be easier to read since it would all be in the same place.
>> >>>>>>>
>> >>>>>>> It would have made sense to keep them separated if there's a
>> >>>>>>> chance
>> >>>>>>> of
>> >>>>>>> someone needing concurrent versions of the chooser UI, which I
>> >>>>>>> don't
>> >>>>>>> think is very probable.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> It would also be nice if we could copy necessary classes from
>> >>>>>>> your
>> >>>>>>>
>> >>>>>>>> lib into the service implementation.
>> >>>>>>>>
>> >>>>>>>> It would be easy to do- autoformatting is a beautiful thing so
>> >>>>>>>> getting
>> >>>>>>>> it to conform to the coding guidelines would be a breeze.
>> >>>>>>>> However,
>> >>>>>>>> unless we're gonna make radical changes to the UI I don't think
>> >>>>>>>> this
>> >>>>>>>> is
>> >>>>>>>> a good idea. Here's a couple of issues:
>> >>>>>>>>
>> >>>>>>>> 1. As the how-to mentions the jar is executable, providing an
>> >>>>>>>> editor
>> >>>>>>>> to
>> >>>>>>>> easily tweak the keybindings. If integrated we'd still probably
>> >>>>>>>> need
>> >>>>>>>> the
>> >>>>>>>> jar for that functionality (with the exception of the
Persistence
>> >>>>>>>> class
>> >>>>>>>> that package probably woulnd't be integrated).
>> >>>>>>>>
>> >>>>>>>> I was actually thinking that if we start saving everything in
>> >>>>>>>> text
>> >>>>>>> form
>> >>>>>>> then we won't need to use the standalone tool when generating or
>> >>>>>>> modifying the default settings.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> 2. As far as hacks go, what's used is pretty short and sweet
>> >>>>>>> (providing
>> >>>>>>>
>> >>>>>>>> an I18N label abstraction and stretching the layout a bit). I
>> >>>>>>>> doubt
>> >>>>>>>> breaking open the jar would clarify anything- it would
certainly
>> >>>>>>>> give
>> >>>>>>>> a
>> >>>>>>>> lot more code to look at.
>> >>>>>>>>
>> >>>>>>>> Well unless I am missing something we'll probably only need
>> >>>>>>>> what's
>> >>>>>>> in
>> >>>>>>> the chooser package which gives us approximately 1400 lines of
>> >>>>>>> code.
>> >>>>>>> The
>> >>>>>>> main reason I am insisting on this is that we'll want to modify
>> >>>>>>> the
>> >>>>>>> UI
>> >>>>>>> and having the code in there would make it easier.
>> >>>>>>>
>> >>>>>>> If you don't feel like making the change then, by all means,
just
>> >>>>>>> leave
>> >>>>>>> it to us. Your contribution is appreciated as it is.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> Then, storing default and user preferences for key bindings
in
>> >>>>>>> a
>> >>>>>>>
>> >>>>>>>> binary form makes it difficult to change them for no real
>> >>>>>>>> reason.
>> >>>>>>>> I've just seen that your lib already allows to store settings
>> >>>>>>>> in a
>> >>>>>>>> properties file so I think this would fit better.
>> >>>>>>>>
>> >>>>>>>> I actually spent quite a while antagonizing over this point.
>> >>>>>>>>
>> >>>>>>>> The point is that we will probably need to modify defaults
quite
>> >>>>>>> often
>> >>>>>>> and not being able to do so by editing a text file would be a
>> >>>>>>> problem.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> The
>> >>>>>>>
>> >>>>>>>> advantage of storing it as a serialized linked hash map is that
>> >>>>>>>> it
>> >>>>>>>> retains ordering information, which is important from a UI
>> >>>>>>>> perspective
>> >>>>>>>> (so options like 'next tab' and 'previous tab' are shown next
to
>> >>>>>>>> each
>> >>>>>>>> other). If that wasn't a concern then certainly- viva la plain
>> >>>>>>>> text.
>> >>>>>>>>
>> >>>>>>>> You can easily implement this with our ConfigurationService by
>> >>>>>>> manually
>> >>>>>>> adding indexes. Here's a very rough example:
>> >>>>>>>
>> >>>>>>> …
>> >>>>>>> <impl>
>> >>>>>>> <keybindings>
>> >>>>>>> <binding1>
>> >>>>>>> <actionName value="Previous Tab"/>
>> >>>>>>> …
>> >>>>>>> </binding1>
>> >>>>>>> <binding2>
>> >>>>>>> <actionName value="Next Tab"/>
>> >>>>>>> …
>> >>>>>>> </binding2>
>> >>>>>>> </keybindings>
>> >>>>>>> </impl>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> We can put the default ones into the defaults.properties
files
>> >>>>>>> and
>> >>>>>>>
>> >>>>>>>> then use the configuration service to modify them.
>> >>>>>>>>
>> >>>>>>>> It looks like this service has changed quite a bit recently.
I'll
>> >>>>>>>> look
>> >>>>>>>> into it.
>> >>>>>>>>
>> >>>>>>>> Yes the defaults are new indeed.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> 1. Would it be possible to implement support for alternative
>> >>>>>>>
>> >>>>>>>> actions? Most key binding management UIs have it would be
>> >>>>>>>> really
>> >>>>>>>> nice if we also did.
>> >>>>>>>>
>> >>>>>>>> I'm not familiar with this. Alternative action of what?
>> >>>>>>>>
>> >>>>>>>> Sorry, my bad, I meant alternative bindings. (See attached
PNG)
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>> >>>>>>> Ctrl-i,
>> >>>>>>>
>> >>>>>>>> Ctrl-u shortcuts that we've recently added to the chat
window?
>> >>>>>>>>
>> >>>>>>>> Oops! That's what I get for failing to sync with the trunk.
>> >>>>>>>> Simple
>> >>>>>>>> once
>> >>>>>>>> I get to a beefier connection.
>> >>>>>>>>
>> >>>>>>>> Thanks, I was hoping you'd say so :wink:
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> Thanks again for the feedback!
>> >>>>>>> Thank you Damian, I do like the feature very much!
>> >>>>>>>
>> >>>>>>> Cheers
>> >>>>>>> Emil
>> >>>>>>>
>> >>>>>>> Cheers,
>> >>>>>>>
>> >>>>>>>> Galen
>> >>>>>>>>
>> >>>>>>>> On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < > >> >>>>>>>> emcho@sip-communicator.org > >> >>>>>>>> > >> >>>>>>>> <mailto:emcho@sip-communicator.org>> wrote:
>> >>>>>>>
>> >>>>>>>> Hey Galen,
>> >>>>>>>>
>> >>>>>>>> Nice work indeed and the feature is quite useful. I've just
>> >>>>>>>> tried
>> >>>>>>>> it
>> >>>>>>>> and
>> >>>>>>>> I like it a lot. Thanks for contributing!
>> >>>>>>>>
>> >>>>>>>> There are however a few things that make me feel
uncomfortable
>> >>>>>>>> with
>> >>>>>>>> the
>> >>>>>>>> way this is currently integrated in SIP Communicator.
>> >>>>>>>>
>> >>>>>>>> First, I don't quite see the need of splitting the
>> >>>>>>>> implementation
>> >>>>>>>> into a
>> >>>>>>>> service and a plugin. The configuration UI could very well
live
>> >>>>>>>> in
>> >>>>>>>> impl.keybinding. It would also be nice if we could copy
>> >>>>>>>> necessary
>> >>>>>>>> classes from your lib into the service implementation. There
>> >>>>>>>> are
>> >>>>>>>> two
>> >>>>>>>> reasons I believe this would a good idea:
>> >>>>>>>> - We'd probably need to make changes on some parts
such
>> >>>>>>>> as
>> >>>>>>>> the
>> >>>>>>>> configuration UI and the persistence mechanisms (see below)
so
>> >>>>>>>> that
>> >>>>>>>> they
>> >>>>>>>> would fit the SC conventions.
>> >>>>>>>> - It would make the code easier to understand.
>> >>>>>>>>
>> >>>>>>>> Then, storing default and user preferences for key bindings
in
>> >>>>>>>> a
>> >>>>>>>> binary
>> >>>>>>>> form makes it difficult to change them for no real reason.
I've
>> >>>>>>>> just
>> >>>>>>>> seen that your lib already allows to store settings in a
>> >>>>>>>> properties
>> >>>>>>>> file
>> >>>>>>>> so I think this would fit better. We can put the default ones
>> >>>>>>>> into
>> >>>>>>>> the
>> >>>>>>>> defaults.properties files and then use the configuration
>> >>>>>>>> service
>> >>>>>>>> to
>> >>>>>>>> modify them.
>> >>>>>>>>
>> >>>>>>>> What do you think?
>> >>>>>>>>
>> >>>>>>>> Other than that I was wondering if I could make the following
>> >>>>>>>> feature
>> >>>>>>>> requests:
>> >>>>>>>>
>> >>>>>>>> 1.Would it be possible to implement support for alternative
>> >>>>>>>> actions?
>> >>>>>>>> Most key binding management UIs have it would be really nice
if
>> >>>>>>>> we
>> >>>>>>>> also did.
>> >>>>>>>> and
>> >>>>>>>> 2. Would you be interested in adding support for the Ctrl-b,
>> >>>>>>>> Ctrl-i,
>> >>>>>>>> Ctrl-u shortcuts that we've recently added to the chat
window?
>> >>>>>>>>
>> >>>>>>>> Once again, thanks for contributing your work!
>> >>>>>>>>
>> >>>>>>>> Emil
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> Yana Stamcheva написа:
>> >>>>>>>> > Hi Galen,
>> >>>>>>>> >
>> >>>>>>>> > you've done a really great job in this patch!
>> >>>>>>>> >
>> >>>>>>>> > First let me apologize for making you wait so long, we were
>> >>>>>>>> really
>> >>>>>>>> busy
>> >>>>>>>> > lately. We have discussed the plugin off list already, but
>> >>>>>>>> it's
>> >>>>>>>> now
>> >>>>>>>> > applied and committed. You're also on our contributors
page.
>> >>>>>>>> >
>> >>>>>>>> > (more inline)
>> >>>>>>>> >
>> >>>>>>>> > Damian Johnson wrote:
>> >>>>>>>> >> Hi. I've just finished integrating a chooser for
keybindings
>> >>>>>>>> into
>> >>>>>>>> the SIP
>> >>>>>>>> >> Communicator. This both provides a means for users to
change
>> >>>>>>>> their
>> >>>>>>>> >> keybinding preferences and save them between runs. Most of
>> >>>>>>>> the
>> >>>>>>>> details of
>> >>>>>>>> >> the addition are described in the latest blog post
>> >>>>>>>> >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
>> >>>>>>>> consists of
>> >>>>>>>> >> two additions:
>> >>>>>>>> >> 1. Keybinding persistence via a new service. This saves
>> >>>>>>>> custom
>> >>>>>>>> bindings with
>> >>>>>>>> >> the user's preferences and informs any frames using the
>> >>>>>>>> bindings
>> >>>>>>>> of updates.
>> >>>>>>>> >> *src/net/java/sip/communicator/impl/keybindings
>> >>>>>>>> >> src/net/java/sip/communicator/service/keybindings
>> >>>>>>>> >
>> >>>>>>>> > Applied. I like the way you have implemented key bindings
in
>> >>>>>>>> the
>> >>>>>>>> gui bundle!
>> >>>>>>>> >
>> >>>>>>>> >> *
>> >>>>>>>> >> 2. Plugin that provides a ConfigurationForm for changing
the
>> >>>>>>>> keybindings.
>> >>>>>>>> >> *src/net/java/sip/communicator/plugin/keybindingChooser*
>> >>>>>>>> >>
>> >>>>>>>> >> I've added a how-to for adding new keybinding sets
>> >>>>>>>> >>
>> >>>>>>>> here<http://www.atagar.com/misc/gsocBlog/keybindings.php
>(below
>> >>>>>>>> the
>> >>>>>>>> >> chooser description). Also, this is my first time making a
>> >>>>>>>> patch
>> >>>>>>>> >> - are binaries supposed to be provided separately like
this?
>> >>>>>>>> I
>> >>>>>>>> didn't spot
>> >>>>>>>> >> an option to include them in svn diff…
>> >>>>>>>> >
>> >>>>>>>> > My reply is coming late, but yes binaries should be
supplied
>> >>>>>>>> separately,
>> >>>>>>>> > because they're not contained in the diff. The how-to is
also
>> >>>>>>>> very
>> >>>>>>>> good
>> >>>>>>>> > idea. Let me know if you're interested in adding this on
the
>> >>>>>>>> SIP
>> >>>>>>>> > Communicator website.
>> >>>>>>>> >
>> >>>>>>>> >> A couple of usability features (any opinions?):
>> >>>>>>>> >> 1. Add a key to revert bindings to defaults. Helpful or
>> >>>>>>>> unnecessary?
>> >>>>>>>> >> 2. Keybindings aren't changed until the user hits "Apply".
>> >>>>>>>> Perhaps bindings
>> >>>>>>>> >> that haven't been applied should be discolored to draw
>> >>>>>>>> attention
>> >>>>>>>> to them?
>> >>>>>>>> >
>> >>>>>>>> > My personal opinion is that a button "Defaults" could be
>> >>>>>>>> really
>> >>>>>>>> helpful.
>> >>>>>>>> > Otherwise coloring unsaved bindings could be useful, but
also
>> >>>>>>>> it
>> >>>>>>>> could
>> >>>>>>>> > be confusing, the user could not be sure what exactly the
>> >>>>>>>> color
>> >>>>>>>> means.
>> >>>>>>>> >
>> >>>>>>>> >> Known Issue:
>> >>>>>>>> >> Not all permissible bindings work in all situations. For
>> >>>>>>>> instance
>> >>>>>>>> in the
>> >>>>>>>> >> main frame the 'Contacts' and 'Chat rooms' panels appear
to
>> >>>>>>>> intercept some
>> >>>>>>>> >> keyboard events, preventing bindings to directional arrows
>> >>>>>>>> (without
>> >>>>>>>> >> modifiers) or the space bar from working. This isn't a
>> >>>>>>>> terribly
>> >>>>>>>> big woop but
>> >>>>>>>> >> might possibly confuse users.
>> >>>>>>>> >
>> >>>>>>>> > Hm, could you explain more in details the problem
situations,
>> >>>>>>>> we
>> >>>>>>>> could
>> >>>>>>>> > try to resolve this.
>> >>>>>>>> >
>> >>>>>>>> >> A few unrelated things I've come across:
>> >>>>>>>> >> 1. The patch also includes the fix for a minor (but very
>> >>>>>>>> consistent)
>> >>>>>>>> >> spelling error: 'desactivate' to 'deactivate'. This
included
>> >>>>>>>> tweaking an
>> >>>>>>>> >> image with the word, changing some language files, and
>> >>>>>>>> refactoring parts of
>> >>>>>>>> >> the ManageButtonsPanel class.
>> >>>>>>>> >
>> >>>>>>>> > Committed all spelling corrections. Thanks.
>> >>>>>>>> >
>> >>>>>>>> >> 2. Does anyone know if there's a known issue with the
>> >>>>>>>> chatalerter
>> >>>>>>>> plugin?
>> >>>>>>>> >> Eclipse has complained that the org.jdesktop.jdic.misc
>> >>>>>>>> package
>> >>>>>>>> can't be
>> >>>>>>>> >> resolved ever since I checked it out via svn. However this
>> >>>>>>>> has
>> >>>>>>>> never
>> >>>>>>>> >> prevented the SIP Communicator from running. I've tried
>> >>>>>>>> asking
>> >>>>>>>> a
>> >>>>>>>> couple of
>> >>>>>>>> >> times on the IRC channel but I think I keep catching
>> >>>>>>>> everyone
>> >>>>>>>> when they're
>> >>>>>>>> >> asleep…
>> >>>>>>>> >
>> >>>>>>>> > Maybe you've already figured it out yourself, but I'm just
>> >>>>>>>> replying for
>> >>>>>>>> > the record…Even that Eclipse is complaining about a
library,
>> >>>>>>>> if
>> >>>>>>>> you
>> >>>>>>>> > compile and run SIP Communicator through Ant it will always
>> >>>>>>>> work,
>> >>>>>>>> > because the classpath is set inside the build.xml. If
Eclipse
>> >>>>>>>> is
>> >>>>>>>> > complaining about a library this means that this library is
>> >>>>>>>> not
>> >>>>>>>> added in
>> >>>>>>>> > the .classpath file of Eclipse. You need to do the
following:
>> >>>>>>>> right
>> >>>>>>>> > click on the project / "Properties" / "Project Build Path"
/
>> >>>>>>>> "Libraries"
>> >>>>>>>> > / "Add new" / Find the library in the lib directory of
SComm.
>> >>>>>>>> >
>> >>>>>>>> >> 3. Couple of minor issues on the site:
>> >>>>>>>> >> Developer Documentation > How to Configure Eclipse
>> >>>>>>>> >> Minor simplification- it's not necessary to get the old
>> >>>>>>>> version
>> >>>>>>>> of Subclipse
>> >>>>>>>> >> then upgrade it. The buckminster error can be avoided by
>> >>>>>>>> disabling the
>> >>>>>>>> >> optional components.
>> >>>>>>>> >
>> >>>>>>>> > Nice catch! I didn't experienced this error, so I'm not
very
>> >>>>>>>> familiar
>> >>>>>>>> > with the problem, so again are you interested in changing
the
>> >>>>>>>> part
>> >>>>>>>> of
>> >>>>>>>> > the tutorial concerning the problem.
>> >>>>>>>> >
>> >>>>>>>> >> Developer Documentation > UI Service > Create and add
>> >>>>>>>> configuration forms
>> >>>>>>>> >> As Atul noticed the 'getConfigurationManager()' method
>> >>>>>>>> doesn't
>> >>>>>>>> exist and
>> >>>>>>>> >> should be 'getConfigurationWindow()'.
>> >>>>>>>> >
>> >>>>>>>> > May be it's too late and I'm almost asleep :slight_smile: but I didn't
>> >>>>>>>> get
>> >>>>>>>> this one,
>> >>>>>>>> > could you be more precise?
>> >>>>>>>> >
>> >>>>>>>> > Damian, the patch was very well documented, very mature and
>> >>>>>>>> easy
>> >>>>>>>> to
>> >>>>>>>> > read. Again you've done a very good work! Thank you for the
>> >>>>>>>> patience and
>> >>>>>>>> > keep up the good work:)
>> >>>>>>>> >
>> >>>>>>>> > Cheers,
>> >>>>>>>> > Yana
>> >>>>>>>> >
>> >>>>>>>> >> Hope this helps. Cheers! -Galen
>> >>>>>>>> >>
>> >>>>>>>> >> PS. I'm planning to try going by my middle name, Galen, in
>> >>>>>>>> SIP
>> >>>>>>>> >> correspondence to avoid confusion with Damien and the
other
>> >>>>>>>> Damian. :slight_smile:
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
------------------------------------------------------------------------
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
------------------------------------------------------------------------
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
------------------------------------------------------------------------
>> >>>>>>>> >>
>> >>>>>>>> >>
>> >>>>>>>>
>> >>>>>>>>
---------------------------------------------------------------------
>> >>>>>>>> >> To unsubscribe, e-mail:
>> >>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>> >>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>> >>>>>>>>
>> >>>>>>>> >> For additional commands, e-mail:
>> >>>>>>>
>> >>>>>>>> dev-help@sip-communicator.dev.java.net
>> >>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>> >>>>>>>>
>> >>>>>>>> >
>> >>>>>>>
>> >>>>>>>> >
>> >>>>>>>>
>> >>>>>>>>
---------------------------------------------------------------------
>> >>>>>>>> > To unsubscribe, e-mail:
>> >>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>> >>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>> >>>>>>>>
>> >>>>>>>> > For additional commands, e-mail:
>> >>>>>>>
>> >>>>>>>> dev-help@sip-communicator.dev.java.net
>> >>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>> >>>>>>>>
>> >>>>>>>> >
>> >>>>>>>
>> >>>>>>>> >
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
---------------------------------------------------------------------
>> >>>>>>>> To unsubscribe, e-mail:
>> >>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>> >>>>>>>> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
>> >>>>>>>>
>> >>>>>>>> For additional commands, e-mail:
>> >>>>>>>
>> >>>>>>>> dev-help@sip-communicator.dev.java.net
>> >>>>>>>> <mailto:dev-help@sip-communicator.dev.java.net>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>
>> >>>>>>>
------------------------------------------------------------------------
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
---------------------------------------------------------------------
>> >>>>>>>> To unsubscribe, e-mail:
>> >>>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>> >>>>>>>> For additional commands, e-mail:
>> >>>>>>>> dev-help@sip-communicator.dev.java.net
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
---------------------------------------------------------------------
>> >>>>>>> To unsubscribe, e-mail:
>> >>>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>> >>>>>>> For additional commands, e-mail:
>> >>>>>>> dev-help@sip-communicator.dev.java.net
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
---------------------------------------------------------------------
>> >>>>>> To unsubscribe, e-mail:
>> >>>>>> dev-unsubscribe@sip-communicator.dev.java.net
>> >>>>>> For additional commands, e-mail:
>> >>>>>> dev-help@sip-communicator.dev.java.net
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
---------------------------------------------------------------------
>> >>>>> To unsubscribe, e-mail:
>> >>>>> dev-unsubscribe@sip-communicator.dev.java.net
>> >>>>> For additional commands, e-mail:
>> >>>>> dev-help@sip-communicator.dev.java.net
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>
---------------------------------------------------------------------
>> >>> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>> >>> For additional commands, e-mail:
>> >>> dev-help@sip-communicator.dev.java.net
>> >>>
>> >>>
>> >>
>> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
> For additional commands, e-mail: dev-help@sip-communicator.dev.java.net
>


#15

Hi Yana. I wouldn't mind dual-licensing KeybindingUtil to avoid the
Apache/LGPL incompatibility issues. This patch has the change.

In the service/gui/gui.manifest.mf you have imported the javax.swing

package. I didn't actually see where you use it inside the service

The issue is with javax.swing.KeyStroke (for the getBindings() method).
Finding an alternative to integrating with the gui service would solve this.

I think we don't ever need the isSettingsStringAvailable(String key) method

in the ResourceManagementService, since we're now returning null if such
string does not exist, so this should be sufficient to check if it's
available.

I agree, but the problem is that the ResourceManagementService logs an error
if the resource can't be found. Hence, when I previously did this the
logging output was filled with hundreds of lines of complaints.

What's the index property about in the configuration properties file? For

example you have
net.java.sip.communicator.impl.gui.keybindings.chat.nextTab.index=0.

The how-to has a description of the fields. This one is for the ordering in
which it's presented in the chooser.

We should think of a method that would allow other bundles to register their

keybindings in the gui. Something like...

We could but there's a couple things I'm concerned about:
1. Programatically registered bindings wouldn't have a default, so what
would the 'Set Defaults' button do to them?
2. Do we really want to provide an alternative to putting all the initial
bindings with the other defaults? Developers would then both need to look at
the defaults and do a grep search through the code to figure out what
bindings are associated with a category.

... also we should trigger events when something has changed...

Could, but I'm having difficulty seeing any advantages since observers and
listeners are functionally equivalent. Listeners are preferred when
triggering UI events while observers concern data in model-view separation
(which it's doing here).

We could move the resetBindings() method into the service but I'm not sure
if hiding this functionality would be wise. It's not always so simple (see
the EditTextToolBar implementation for instance). Also, when there's
unconfigurable bindings (such as 'escape -> close' with the MainFrame or
'shift + backspace -> deletePrevCharAction' with the EditTextToolBar) this
is where they're added.

Thanks for the feedback. Cheers! -Damian

keybindingsUpdate3.patch (178 KB)

···

On Tue, Aug 5, 2008 at 4:19 AM, Yana Stamcheva <yana@sip-communicator.org>wrote:

Hi Damian,

The screenshot is very nice!

However, in order to be able to commit your patch we would need you to
change the license of your classes to the SIP Communicator license, instead
of the Apache v2.

Otherwise I have also some questions:

- In the service/gui/gui.manifest.mf you have imported the javax.swing
package. I didn't actually see where you use it inside the service, but
we're avoiding putting in the service the swing package, because this would
limit us to have only swing implementations of the service.

- I think we don't ever need the isSettingsStringAvailable(String key)
method in the ResourceManagementService, since we're now returning null if
such string does not exist, so this should be sufficient to check if it's
available.

- What's the index property about in the configuration properties file? For
example you have
net.java.sip.communicator.impl.gui.keybindings.chat.nextTab.index=0.

- Just some more thoughts.. We should think of a method that would allow
other bundles to register their keybindings in the gui. Something like:

registerKeybinding(Category, KeyStroke, String);

and also we should trigger events when something has changed:

addKeybindingListener(KeybindingListener l);

We could then use these instead of the Observer and the resetBindings
method.

Cheers,
Yana

Damian Johnson wrote:

Hi. I've just finished implementing all the requested changes for the
keybindings service. In addition to the previous change list there's now
alternative binding support (see attached screenshot), the jar's been
unpacked (and removed) and the service no longer uses binaries for default
or custom bindings. The following example uses the bindings added for the
chat toolbar.

In defaults.properties the binding information looks like:
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.index=0

net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcut=ctrl
pressed B

net.java.sip.communicator.impl.gui.keybindings.chatToolbar.bold.shortcutAlt=ctrl
pressed F
net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.index=1

net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcut=ctrl
pressed I

net.java.sip.communicator.impl.gui.keybindings.chatToolbar.italic.shortcutAlt=ctrl
pressed K

net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.index=2

net.java.sip.communicator.impl.gui.keybindings.chatToolbar.underline.shortcut=ctrl
pressed U

Field's functions should be self explanatory and the alternate shortcuts
('shortcutAlt') are optional. Custom bindings are stored via the
ConfigurationService (in my case to
~/.sip-communicator/sip-communicator.xml) and look something like:
<keybindings>
<chatToolbar>
   <bold value="ctrl pressed B"/>
   <bold-alt value="ctrl pressed F"/>
   <italic value="ctrl pressed I"/>
   <italic-alt value="ctrl pressed K"/>
   <underline value="ctrl pressed U"/>
   <underline-alt value=""/>
</chatToolbar>
</keybindings>

Quite a bit was removed and I'm not sure if the patch reflects that so
I've
included the svn status output to give an indication of where changes were
made. Unlike the normal bindings, the alternatives can be removed (with
the
escape key). One other note is that I added an
'isSettingsStringAvailable(String)' method to the
ResourceManagementService
since otherwise there's no means of figuring out if a default exists.
Personally I think the getSettingsString(String) method should throw the
MissingResourceException rather than log errors (and have a javadoc
description) but I'm not sure if changing this would upset other classes
that currently use it. Cheers! -Damian

2008/7/27 Emil Ivov <emcho@sip-communicator.org>

Hey Damian,

Glad to hear you are making progress on this one!

The defaults.properties file was indeed previewed for cases just like
yours: the conf service is supposed to load it as default content the
first time it runs.

However, as you have noticed this behaviour hasn't been fully
implemented yet so right now you can either implement it yourself or
do as Damien suggested - use a resource file inside your bundle and in
case your properties are not already in the ConfigurationService, load
them in there by your self.

Cheers
Emil

On 7/27/08, Damian Johnson <atagar1@gmail.com> wrote:

Hi. I've tweaked the KeybindingsService implementation to utilize the
ConfigurationService so the custom bindings are now stored in a
human-editable fashion. However, I'm having some difficulty finding
where
the defaults should be stored. The obvious choice is
'resources/config/defaults.properties' but the
ResourceManagementService,
unlike ConfigurationService, isn't structured as a tree and hence
doesn't
have any methods for retrieving batches of defaults (ie, all chat
keybindings). Per chance is there a file with the initial values of
'~/.sip-communicator/sip-communicator.xml'? -Damian

2008/7/25 Damian Johnson <atagar1@gmail.com>

Hi Yana. Thanks for the quick response! Previous discussion on this

thread

concerned the binary property files. I chose serialized linked hash maps

so
ordering would be preserved (which is important for the appearance).
Emil's
suggested adding an index to the plain text properties and I'm planning

to

look into this soon. -Damian

2008/7/25 Yana Stamcheva <yana@sip-communicator.org>:

Hi Damian,

Damian Johnson wrote:

Hi. Thus far I've made the following changes:

Added 'Set Default' button
Added bindings for fonts
Merged keybindingchoosr plugin into keybindings service
implementation
Bug fix: Issue with the escape binding (thanks, Pavel, for catching
that!)
Bug fix (*impl/gui/main/chat/toolBars/EditTextToolBar.java*):
NullPointerException if user cancels the color chooser
Bug fix: Added 'lib/installer-exclude/jfontchooser-1.0.5.jar' and
'lib/os-specific/mac/installer-exclude/dock.jar' to ide classpaths

Good job!

However, I've come up against a bit of a chicken-and-egg problem. The

swing-ui bundle's activator uses the keybindings service to
initialize
the
UI, hence that service needs to start first. However, the keybinding
chooser
needs the swing-ui to set up the custom look-and-feel (the
ConfigurationForm
is using the swing default now that it's starting first).

I think we should merge the keybindings service into swing-ui (*
/impl/keybindings* to */impl/gui/keybindings*) for a couple reasons
beyond
this issue:
1. Currently the KeybindingsService is being provided through the
GuiActivator (part of the swing-ui bundle). Actually, the keybindings
service isn't available to plugins unless we're gonna make the
service
available through something like the UIService.
2. Keybindings are strictly a UI feature anyway...

Does this sound good? If not, any thoughts on alternatives? Cheers!
-Damian

Sounds very good. I have also discussed this with Emil off list and

he
also agrees, so you could go ahead and integrate it in the swing-ui.

Just one more thing. I'm not sure if Emil has already asked, but is

there

any special reason for which the configuration files for keybindings :

keybindings-chat and keybindings-main are binary files? Could we make
them
just properties files, so that they're easier to read?

Cheers,
Yana

2008/7/22 Yana Stamcheva <yana@sip-communicator.org>:

Hi Pavel,

I have committed a fix and the font button should be now working

(from

build 1242). Let me know if you continue experiencing problems.

Cheers,
Yana

Yana Stamcheva wrote:

Hi Pavel,

thanks for reporting!

I think that only the Escape key problem is related to Damian's

work,

but

I'm confirming the two other problems.

I'll have a look at the font button asap.

Is there someone else experiencing encoding problems when
sending/receiving messages ? The first thing that comes to me is

that

it

could be due to recent modifications for HTML message support.

Cheers,
Yana

Pavel Tankov wrote:

Hello Damian,

It looks like there are a few problems now with the new keybinding
plugin. Here are a few I've noticed that worked before, but now
don't:
- Escape key doesn't close chat window;
- The button for selecting font doesn't popup the selection
window;
- My messages sent in cyrillic appear as ??? on the other side.
Probably that's true for other non-latin encodings.

Thanks,
Pavel Tankov

----- Original Message ----
From: Damian Johnson <atagar1@gmail.com>
To: dev@sip-communicator.dev.java.net
Sent: Saturday, July 19, 2008 9:22:57 AM
Subject: Re: [sip-comm-dev] Keybinding Chooser Addition

Hi Emil,

I am not sure I see why this prevents us from copying the content

of

the

plugin.keybindingchooser package into impl.keybindings.
Ack! Sorry, I misread your original comment and thought the
proposition
was to do away with the service. My bad.
I was trying to say that since the configuration UI is quite

tightly

coupled with this service implementation and both are quite light

then
they
probably should belong to the same package.... It would have made
sense to
keep them separated if there's a chance of someone needing

concurrent

versions of the chooser UI, which I don't think is very probable.

Good points- I'll make the change.
Well unless I am missing something we'll probably only need
what's
in
the chooser package which gives us approximately 1400 lines of

code.

The

main reason I am insisting on this is that we'll want to modify
the
UI
and
having the code in there would make it easier... If you don't feel
like
making the change then, by all means, just leave it to us. Your
contribution
is appreciated as it is
As I mentioned if we're gonna make substantial changes to the UI

then

this makes sense, and the 'alternative bindings' are certainly

substantial
enough. I'm sorry if I gave the impression that I wasn't willing
to
make the
change- I'll handle it.
... I meant alternative bindings
I've never seen this feature before but should be simple once the
chooser's source is added in.
Hopefully we'll soon be able to start integrating the spell

checker

soon

too. :slight_smile:
Cheers,
Damian
PS. I give up on going by 'Galen'. With webmail metadata adding
in
my
name this is just confusing. Oh well- it was worth a try...
On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov < >>>>>>>>>> emcho@sip-communicator.org> >>>>>>>>>> wrote:

Hey Damian,

Damian Johnson написа:

  First, I don't quite see the need of splitting the

implementation

into a service and a plugin.

Actually I was just thinking about this. Are we going to want
plugins
to
be able to listen for and process keyboard events?

Most probably.

If so then the

addition shouldn't use the new methods provided via the

SIPCommFrame.
If
not then you're completely right.

I am not sure I see why this prevents us from copying the
content
of

the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since
the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should
belong
to the same package.

  The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making

alternative choosers (which I doubt) there's no need for it to
act
as
a
plugin.

I agree, developers are likely to tackle the UI but since both

the

plugin and the impl are quite light, merging them would not make

modifying the UI any more difficult. Quite, on the contrary, code
would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a
chance
of
someone needing concurrent versions of the chooser UI, which I

don't

think is very probable.

  It would also be nice if we could copy necessary classes from

your

lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so
getting
it to conform to the coding guidelines would be a breeze.
However,
unless we're gonna make radical changes to the UI I don't think

this

is

a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an

editor

to

easily tweak the keybindings. If integrated we'd still probably

need

the

jar for that functionality (with the exception of the Persistence
class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in

text

form

then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet
(providing

an I18N label abstraction and stretching the layout a bit). I

doubt

breaking open the jar would clarify anything- it would certainly

give
a
lot more code to look at.

Well unless I am missing something we'll probably only need

what's

in

the chooser package which gives us approximately 1400 lines of

code.

The

main reason I am insisting on this is that we'll want to modify
the
UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just
leave
it to us. Your contribution is appreciated as it is.

  Then, storing default and user preferences for key bindings in a

  binary form makes it difficult to change them for no real

reason.

I've just seen that your lib already allows to store settings in

a

properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite

often
and not being able to do so by editing a text file would be a
problem.

The

advantage of storing it as a serialized linked hash map is that

it
retains ordering information, which is important from a UI
perspective
(so options like 'next tab' and 'previous tab' are shown next to
each
other). If that wasn't a concern then certainly- viva la plain

text.

You can easily implement this with our ConfigurationService by

manually
adding indexes. Here's a very rough example:

....
<impl>
<keybindings>
    <binding1>
        <actionName value="Previous Tab"/>
        ...
    </binding1>
    <binding2>
        <actionName value="Next Tab"/>
        ...
    </binding2>
</keybindings>
</impl>

  We can put the default ones into the defaults.properties files

and

then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll
look
into it.

Yes the defaults are new indeed.

  1. Would it be possible to implement support for alternative

  actions? Most key binding management UIs have it would be really

nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

  2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,

  Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk.
Simple
once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!
Thank you Damian, I do like the feature very much!

Cheers
Emil

Cheers,

Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov < >>>>>>>>>>> emcho@sip-communicator.org >>>>>>>>>>> >>>>>>>>>>> <mailto:emcho@sip-communicator.org>> wrote:
Hey Galen,

Nice work indeed and the feature is quite useful. I've just

tried

it

and
I like it a lot. Thanks for contributing!

There are however a few things that make me feel uncomfortable
with
the
way this is currently integrated in SIP Communicator.

First, I don't quite see the need of splitting the

implementation

into a

service and a plugin. The configuration UI could very well live

in

impl.keybinding. It would also be nice if we could copy

necessary

classes from your lib into the service implementation. There are

two
reasons I believe this would a good idea:
        - We'd probably need to make changes on some parts such

as

the

configuration UI and the persistence mechanisms (see below) so
that
they
would fit the SC conventions.
        - It would make the code easier to understand.

Then, storing default and user preferences for key bindings in a
binary
form makes it difficult to change them for no real reason. I've
just
seen that your lib already allows to store settings in a
properties
file
so I think this would fit better. We can put the default ones

into

the

defaults.properties files and then use the configuration service
to
modify them.

What do you think?

Other than that I was wondering if I could make the following
feature
requests:

1.Would it be possible to implement support for alternative
actions?
Most key binding management UIs have it would be really nice if

we

also did.

and
2. Would you be interested in adding support for the Ctrl-b,
Ctrl-i,
Ctrl-u shortcuts that we've recently added to the chat window?

Once again, thanks for contributing your work!

Emil

Yana Stamcheva написа:
> Hi Galen,
>
> you've done a really great job in this patch!
>
> First let me apologize for making you wait so long, we were
really
busy
> lately. We have discussed the plugin off list already, but

it's

now

> applied and committed. You're also on our contributors page.
>
> (more inline)
>
> Damian Johnson wrote:
>> Hi. I've just finished integrating a chooser for keybindings
into
the SIP
>> Communicator. This both provides a means for users to change
their
>> keybinding preferences and save them between runs. Most of

the

details of

>> the addition are described in the latest blog post
>> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
>> two additions:
>> 1. Keybinding persistence via a new service. This saves

custom

bindings with

>> the user's preferences and informs any frames using the
bindings
of updates.
>> *src/net/java/sip/communicator/impl/keybindings
>> src/net/java/sip/communicator/service/keybindings
>
> Applied. I like the way you have implemented key bindings in

the

gui bundle!

>
>> *
>> 2. Plugin that provides a ConfigurationForm for changing the
keybindings.
>> *src/net/java/sip/communicator/plugin/keybindingChooser*
>>
>> I've added a how-to for adding new keybinding sets
>> here<http://www.atagar.com/misc/gsocBlog/keybindings.php

(below

the

>> chooser description). Also, this is my first time making a
patch
>> - are binaries supposed to be provided separately like this?

I

didn't spot

>> an option to include them in svn diff...
>
> My reply is coming late, but yes binaries should be supplied
separately,
> because they're not contained in the diff. The how-to is also
very
good
> idea. Let me know if you're interested in adding this on the

SIP

> Communicator website.

>
>> A couple of usability features (any opinions?):
>> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
>> 2. Keybindings aren't changed until the user hits "Apply".
Perhaps bindings
>> that haven't been applied should be discolored to draw
attention
to them?
>
> My personal opinion is that a button "Defaults" could be

really

helpful.

> Otherwise coloring unsaved bindings could be useful, but also

it

could

> be confusing, the user could not be sure what exactly the

color

means.

>
>> Known Issue:
>> Not all permissible bindings work in all situations. For
instance
in the
>> main frame the 'Contacts' and 'Chat rooms' panels appear to
intercept some
>> keyboard events, preventing bindings to directional arrows
(without
>> modifiers) or the space bar from working. This isn't a

terribly

big woop but

>> might possibly confuse users.
>
> Hm, could you explain more in details the problem situations,

we

could

> try to resolve this.
>
>> A few unrelated things I've come across:
>> 1. The patch also includes the fix for a minor (but very
consistent)
>> spelling error: 'desactivate' to 'deactivate'. This included
tweaking an
>> image with the word, changing some language files, and
refactoring parts of
>> the ManageButtonsPanel class.
>
> Committed all spelling corrections. Thanks.
>
>> 2. Does anyone know if there's a known issue with the
chatalerter
plugin?
>> Eclipse has complained that the org.jdesktop.jdic.misc

package

can't be

>> resolved ever since I checked it out via svn. However this

has

never

>> prevented the SIP Communicator from running. I've tried

asking

a

couple of
>> times on the IRC channel but I think I keep catching everyone
when they're
>> asleep...
>
> Maybe you've already figured it out yourself, but I'm just
replying for
> the record..Even that Eclipse is complaining about a library,

if

you

> compile and run SIP Communicator through Ant it will always
work,
> because the classpath is set inside the build.xml. If Eclipse

is

> complaining about a library this means that this library is

not

added in

> the .classpath file of Eclipse. You need to do the following:
right
> click on the project / "Properties" / "Project Build Path" /
"Libraries"
> / "Add new" / Find the library in the lib directory of SComm.
>
>> 3. Couple of minor issues on the site:
>> Developer Documentation > How to Configure Eclipse
>> Minor simplification- it's not necessary to get the old

version

of Subclipse

>> then upgrade it. The buckminster error can be avoided by
disabling the
>> optional components.
>
> Nice catch! I didn't experienced this error, so I'm not very
familiar
> with the problem, so again are you interested in changing the
part
of
> the tutorial concerning the problem.
>
>> Developer Documentation > UI Service > Create and add
configuration forms
>> As Atul noticed the 'getConfigurationManager()' method

doesn't

exist and

>> should be 'getConfigurationWindow()'.
>
> May be it's too late and I'm almost asleep :slight_smile: but I didn't get
this one,
> could you be more precise?
>
> Damian, the patch was very well documented, very mature and

easy

to

> read. Again you've done a very good work! Thank you for the
patience and
> keep up the good work:)
>
> Cheers,
> Yana
>
>> Hope this helps. Cheers! -Galen
>>
>> PS. I'm planning to try going by my middle name, Galen, in

SIP

>> correspondence to avoid confusion with Damien and the other

Damian. :slight_smile:
>>
>>
>>
>>

------------------------------------------------------------------------

>>

>>
>>

------------------------------------------------------------------------

>>

>>
>>

------------------------------------------------------------------------

>>

>>

---------------------------------------------------------------------

>> To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net
<mailto:dev-unsubscribe@sip-communicator.dev.java.net>

   >> For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
<mailto:dev-help@sip-communicator.dev.java.net>

   >
>

---------------------------------------------------------------------

> To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net
<mailto:dev-unsubscribe@sip-communicator.dev.java.net>

   > For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
<mailto:dev-help@sip-communicator.dev.java.net>

   >
>

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net
<mailto:dev-unsubscribe@sip-communicator.dev.java.net>

   For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
<mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

------------------------------------------------------------------------

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net