Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LCID for ProxyObject is hardcoded and inconsistent #659

Closed
matthiasblaesing opened this issue May 16, 2016 · 3 comments
Closed

LCID for ProxyObject is hardcoded and inconsistent #659

matthiasblaesing opened this issue May 16, 2016 · 3 comments

Comments

@matthiasblaesing
Copy link
Member

@dhakehurst while working with excel I noted, that I'd need to change th used LCID for the COM invocations. Excel formulas are interpreted locale dependent and here unifying to the english locale helps development. I already overrid the LCID manually and got the expected locale (I'm on german locale, and could now send english formulas).

While looking through the ProxyObject code, I noticed, that you decided to use different default LCIDs:

GetIDsOfNames is using LOCALE_USER_DEFAULT
Invoke is called with LOCALE_SYSTEM_DEFAULT

From my perspective this looks inconsistent. Is there a reason for this?

I'd like to make the locale configurable, my idea:

  • add a "getLCID" method to the factory (by default reporting what is today reported as LOCALE_USER_DEFAULT)
  • add a "setLCID" method to the factory to be able to override the method
  • make ProxyObject use getLCID to determine the LCID to use

The big question: would this be enough and workable or are more complex methods needed? Instead of a fixed LCID I could imaging an LCIDMapper, that takes a target object and java.lang.reflect.Method and returns the LCID to use.

@matthiasblaesing matthiasblaesing changed the title LCID for ProxyObject is hardcoded and incosistent LCID for ProxyObject is hardcoded and inconsistent May 16, 2016
@dhakehurst
Copy link

Your suggestion seems fine to me.
I don't remember a reason for picking one LCID over another.

On 16 May 2016 at 21:16, matthiasblaesing notifications@github.com wrote:

@dhakehurst https://github.com/dhakehurst while working with excel I
noted, that I'd need to change th used LCID for the COM invocations. Excel
formulas are interpreted locale dependent and here unifying to the english
locale helps development. I already overrid the LCID manually and got the
expected locale (I'm on german locale, and could now send english formulas).

While looking through the ProxyObject code, I noticed, that you decided to
use different default LCIDs:

GetIDsOfNames is using LOCALE_USER_DEFAULT
Invoke is called with LOCALE_SYSTEM_DEFAULT

From my perspective this looks inconsistent. Is there a reason for this?

I'd like to make the locale configurable, my idea:

  • add a "getLCID" method to the factory (by default reporting what is
    today reported as LOCALE_USER_DEFAULT)
  • add a "setLCID" method to the factory to be able to override the
    method
  • make ProxyObject use getLCID to determine the LCID to use

The big question: would this be enough and workable or are more complex
methods needed? Instead of a fixed LCID I could imaging an LCIDMapper, that
takes a target object and java.lang.reflect.Method and returns the LCID to
use.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#659

matthiasblaesing added a commit to matthiasblaesing/jna that referenced this issue May 19, 2016
The system/user default LCID is not always the best/correct value to
use for COM invocations. One example is excel, if the default user
LCID is used, formulas need to be coded in the language of the user.

This commit moves the LCID handling into the factory, so LCID can be
overridden per instantiated factory.

A unittest was added, but marked as ignored, as the test is depending
on the installed excel locale and only usable on non-english locales.
(test was run on a german excel version)

In addition to the unittest the msoffice sample was adjusted to be
usable on non-english locales (manually tested on same german excel
version specified above).

Closes: java-native-access#659
matthiasblaesing added a commit to matthiasblaesing/jna that referenced this issue May 20, 2016
The system/user default LCID is not always the best/correct value to
use for COM invocations. One example is excel, if the default user
LCID is used, formulas need to be coded in the language of the user.

This commit moves the LCID handling into the factory, so LCID can be
overridden per instantiated factory.

A unittest was added, but marked as ignored, as the test is depending
on the installed excel locale and only usable on non-english locales.
(test was run on a german excel version)

In addition to the unittest the msoffice sample was adjusted to be
usable on non-english locales (manually tested on same german excel
version specified above).

Closes: java-native-access#659
@matthiasblaesing
Copy link
Member Author

@dhakehurst I implemented it as described. I pushed a sample branch:

https://github.com/matthiasblaesing/jna/tree/simple_com_thread_2

this branch holds the reintroduction of the simplified threading factory and the LCID change. I'd appreciate it, if you could have a look at it.

@dhakehurst
Copy link

I will look when I can, am in the middle of a different project at present,
so it will be a couple of weeks before I can look.

Many thanks for this.

On 23 May 2016 at 20:21, matthiasblaesing notifications@github.com wrote:

@dhakehurst https://github.com/dhakehurst I implemented it as
described. I pushed a sample branch:

https://github.com/matthiasblaesing/jna/tree/simple_com_thread_2

this branch holds the reintroduction of the simplified threading factory
and the LCID change. I'd appreciate it, if you could have a look at it.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#659 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants