This repository was archived by the owner on Feb 26, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Feb 26, 2023. It is now read-only.
Introduce possibility to change GENERATION_SUFFIX #1209
Description
I would be nice to be able to use a generation suffix other then "underscore" - for classes only of course, class members can have the current one. The quick and dirty solution could be to configure it via compilerArgs and OptionsHelper. The better (but more complicated) would be to configure it on class level with AA annotations - e.g @ebean(suffix="Impl") which will produce MyBeanImpl instead of MyBean_ . I know it sounds stupid but it would be great to have more canonical class names inside some reusable lib. What do you think?
Activity
WonderCsabo commentedon Nov 5, 2014
I like the
OptionsHelperway much more, because it retains consistency of the naming above classes, and does not add an extra parameter to every class annotation.But i am not sure whether this feature is needed or not. @yDelouis @DayS what do you think?
chpasha commentedon Nov 5, 2014
I agree, it needs much less work, not so flexible but better than nothing. In some shared lib suffix like Impl would do
WonderCsabo commentedon Nov 5, 2014
BTW, this change is still problematic.
GENERATION_SUFFIXfor methods and field names, sometimes. It is not a good idea. Since these names are not interesting, we could just hardcode_, or better, create another constant for those. Replacing all_s with the new constant also requires some work.GENERATION_SUFFIXeverywhere it needed. So if we make this change, we have to read the code in detail, and also test every possible case... Searching for_can help.chpasha commentedon Nov 5, 2014
Tried to make a change as a proof of concept, seems to work. There are a couple of places in helper classes where GENERATION_SUFFIX is used and no processingenvironment is available (e.g. AnnotationHelper.generatedClassQualifiedNameFromQualifiedName) so it is possible that for some annotations the old name will be still generated, but all inheritors of BaseGeneratedClassHolder should be fine (of course my knowledge of the AA internals is poor :) - I look into the code for the second time in my life)
P.S. ok, to get appropriate suffix in helper classes is also not a problem
P.P.S. except for the validation part which issues false warnings about not registered component :(
P.P.P.S also not a problem, needs ProcessingEnvironment as constructor arg
WonderCsabo commentedon Nov 5, 2014
I suggest to read the option at processing start and set GENERATION_SUFFIX
there.
chpasha commentedon Nov 5, 2014
nice idea. the probability to break something could be higher in this case though, isn't it? there are most probably some places, where underscore is used explicitly. Besides that all methods and fields will become the suffix (which is not a tragedy I think)
WonderCsabo commentedon Nov 5, 2014
Actually I think it is lower, but that does not matter, since the whole
codebase has to be searched for the underscore anyway, and replace that
with the constant if needed.
The field and method names should use a different constant.
WonderCsabo commentedon Nov 5, 2014
BTW, if you started work on this and have questions about the implementation, it is better to open a PR even for the WIP code, so we can discuss that more easily.
chpasha commentedon Nov 5, 2014
for now I've made changes only in my private (already slightly modified) repository. I will have to learn myself this pull request thing first, to be able to contribute.
WonderCsabo commentedon Nov 5, 2014
You can read the GH help page about pull requests. Also please read our guide about contributing (i assume you already read the environment initialization). To be able to contribute, you have to gain some basic git knowledge. The official git webpage is a great starting point, and i suggest to read at least the first three chapters of the git book.
chpasha commentedon Nov 5, 2014
I will, thanks
WonderCsabo commentedon Nov 21, 2014
@chpasha did you manage to work on this?
chpasha commentedon Nov 21, 2014
not yet, sry
dodgex commentedon Dec 22, 2014
there is a PR now.
WonderCsabo commentedon May 3, 2015
Implemented.