-
Notifications
You must be signed in to change notification settings - Fork 66
hyphen-case config key support #12
base: master
Are you sure you want to change the base?
Conversation
Thanks, @frohoff! This is a useful addition. I really like the idea of passing a name transformation into Here are a couple thoughts, though. Currently Ficus's only runtime dependency is Typesafe config, and I'd prefer to keep it that way. I've gotten into a dependency nightmare with Guava version mismatches before, and they are no fun. Separately, it doesn't seem to me that the Therefore, I propose that we do the following:
What do you think? |
I had actually originally intended to implement with a user-specifiable mapping function it similar to how you suggested but it was seeming quite complicated to pass parameters through the macro definition into the implementation and helper methods, though my experience with macros is pretty limited and it's possible there's something I'm not considering. With this in mind, I was hesitant to do anything too convoluted in the implementation, and without a mechanism to pass parameters through, it would probably leave ficus library users to write their own macro definitions which seemed like something one would want to avoid. I'm definitely open to suggestions/direction. |
Ah, thanks for the explanation. I didn't realize it was so difficult to pass custom arguments to macros. I'm wondering if we could still use String => String without NameMapper though it looks like we will still have to do it via macro like you have said. I'm still pretty hesitant to introduce the Guava dependency. Maybe it would make sense to have a separate module that could add the Guava dependency and have helpers for a few different formats? I don't know, I'll ponder this a bit. |
Agreed on dropping |
I would say don't sink too much time using the annotation approach for now. It's complicated and probably less flexible than other approaches. I have some ideas I would like to try out, but it might be a few days before I get to it. |
One nice thing about the annotation approach is that it might enable further customization of these readers for things like the exhaustive key-parameter mapping I described in #13. |
I threw together a rough working prototype that supports customizable parameter mapping and exhaustivity checking (for #13) using annotations at frohoff/ficus@54cb7d1 but then had another idea: Another way to do this that seemed a bit cleaner was to have the macro compile in calls/references to members on the trait/object it's being called "through" (the macro's A rough working prototype for this is at frohoff/ficus@57de54d This ends up looking roughly like this: // user code
object CustomizedArbitraryTypeReader extends ArbitraryTypeReader {
override val checkExhaustivity: Boolean = true
override def mapParams(n: String) = CaseFormat.LOWER_CAMEL.to(CaseFormat.LOWER_HYPHEN, n)
}
// library code
trait ArbitraryTypeReader {
implicit def arbitraryTypeValueReader[T]: ValueReader[T] = macro ArbitraryTypeReaderMacros.arbitraryTypeValueReader[T]
val checkExhaustivity: Boolean = false
def mapParams(n: String) = n
} This should also allow implementing common options as modular traits a la: // user code
object MyArbitraryTypeReader
extends ArbitraryTypeReader
with GuavaHyphenParamMapping // optional guava dependency or in separate module
with ExhaustivityChecking |
Looking forward for this to be merged. |
This is a draft PR to implement #7 using Guava's
CaseFormat
class for the camelCase to hyphen-case conversion.Created separate
HyphenCaseArbitraryTypeReader
trait and object (existingArbitraryTypeReader
API/behavior is unchanged) that invokes a new macro that specifies a hyphen-caseNameMapper
trait implementation that gets passed down to theextractMethodArgsFromConfig()
method. Added a test case toArbitraryTypeReaderSpec
.Probably could be reorganized a bit. Please advise.