Summary: | NamingConventions.suggestVariableNames doesn't work if name contains '_' | ||||||
---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Dani Megert <daniel_megert> | ||||
Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||
Status: | VERIFIED FIXED | QA Contact: | |||||
Severity: | normal | ||||||
Priority: | P3 | CC: | david_audel, markus.kell.r, Olivier_Thomann, srikanth_sankaran | ||||
Version: | 3.5 | ||||||
Target Milestone: | 3.6 M3 | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Dani Megert
2009-07-15 08:39:40 EDT
So you expect the '_' character to be there in the proposals ? Right, that's what I would expect from reading the Javadoc. This might impact existing users. So we might want to consider fixing the documentation as well. >So we might want to consider fixing the documentation as well.
You mean instead of fixing the bug?
Yes. If existing users have some code that relies on removing the '_', they would be broken once this is fixed. The API was made on request from JDT UI and most likely they are the only clients of it and currently it's breaking their features. If JDT Core only intends to update the doc then you have to provide a new working API for JDT UI and deprecate the old one. Ok, I was not aware of this. We should then fix the bug. So, what do we expect for inputs like these - "MY_FIRST_Type" and "my_first_Type". Right now the output is {"myFIRSTType", "firstType","type"} and {"myFirstType", "firstType", "type"} respectively. I think we need to come up with the scenarios where '_' will be considered a delimiter. I don't expect any special processing of '_', since '_' has no special meaning inside a base name. If the original name contains '_' and is the name of a constant, clients should already have passed it to NamingConventions.getBaseName(NamingConventions.VK_STATIC_FINAL_FIELD, ..) to convert it into a baseName and get rid of the '_'s. So, I expect what the Javadoc says: - "lMinTriggerPeriod_usec" -> "lMinTriggerPeriod_usec", "minTriggerPeriod_usec", "triggerPeriod_usec", "period_usec" - "MY_FIRST_Type" -> "mY_FIRST_Type" - "my_first_Type" -> "my_first_Type" As an alternative, you could also consider '_' as a word-separator, i.e.: - "lMinTriggerPeriod_usec" -> "lMinTriggerPeriod_usec", "minTriggerPeriod_usec", "triggerPeriod_usec", "period_usec", "usec" - "MY_FIRST_Type" -> "mY_FIRST_Type", "fIRST_TYPE", "tYPE" - "my_first_Type" -> "my_first_Type", "first_Type", "type" I had a look at the NamingConvensions code and the tests and it looks like the code has been written to convert the first word to lower case instead of the first letter, as mentioned in the doc. I just confirmed this with David. So, do we want to keep this behavior and fix the '_' problem? This will produce outputs like these: MY_FIRST_Type -> "my_FIRST_Type", "first_Type", "type" my_first_Type -> "my_First_Type", "first_Type", "type" That would also be fine with me, iff you fix the Javadoc and
> my_first_Type -> "my_First_Type", [..]
was a typo and really meant:
-> "my_first_Type"
Created attachment 147076 [details]
Proposed Patch
Attaching the patch with documentation change and tests. The fix required changes to some of the existing tests.
I didn't verify the patch in detail but the general direction looks good and I verified that - it fixes the original issue from bug 263058 comment 6 - our JDT Text and JDT UI tests still pass Released in HEAD for 3.6M3 (after minor indentation changes) Set target milestone. Verified for 3.6M3 using build I20091026-2000 |