Skip to content

Change to suggested plugin style#1

Open
cvogt wants to merge 3 commits into
megri:masterfrom
cvogt:splain-2
Open

Change to suggested plugin style#1
cvogt wants to merge 3 commits into
megri:masterfrom
cvogt:splain-2

Conversation

@cvogt
Copy link
Copy Markdown

@cvogt cvogt commented Jun 15, 2017

@megri how do you feel about this?

)(
implicit logger: Logger, transientCache: java.util.Map[AnyRef,AnyRef], classLoaderCache: ClassLoaderCache
){
case class config( version: String = "0.2.4", options: Seq[SplainOption] = Seq( all() ) ){
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this case class apply and a nested case class config is a bit unusual, but allows for some very nice properties as far as I can tell. Many of the non stage2 plugins are following this style for a while. (Also see doc/plugin-author-guide.md for more info about this pattern). But basically it allows plugins to occupy only a single field in the trait and allowing for it to be configurable by:

  override def splain = super.splain.copy(
   version = "0.2.5",
   options = super.splain.options :+ Splain.color(false)
  )

it also now allows less prefixing names with "splain", because they are already scoped in the relevant object/class as members or arguments.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case class apply is basically for argument that shouldn't be changed by the user. case class config is for those that should. sort of private / public.

* A scala compiler plugin for more concise errors
* See: https://github.com/tek/splain
*/
trait Splain extends BaseBuild{
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

none of the other traits end in Plugin right now, so I removed that


object Splain{
abstract class SplainOption( name: String, value: String ){
def scalacOption = "-P:splain:" ~ name ~ ":" ~ value
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

~ is cbt's type-safe string +

case class tree( value: Boolean = true ) extends SplainOption( "tree", value.toString )
case class compact( value: Boolean = false ) extends SplainOption( "compact", if( value ) "on" else "off" )
case class boundsimplicits( value: Boolean = true ) extends SplainOption( "boundsimplicits", value.toString )
case class truncrefined( value: Int = 0 ) extends SplainOption( "truncrefined", value.toString )
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower cased them to be consistent how splain calls them. Not 100% sure about this, but tend to rather adhere to the pattern of the library instead of Scala's kindof fragile naming, where terms are sometimes lower case (defs, vals) and sometimes upper case (objects), but all can behave like functions.

@cvogt
Copy link
Copy Markdown
Author

cvogt commented Jun 15, 2017

an example/test demonstrating it actually works would be great for this one as well

@megri megri force-pushed the master branch 3 times, most recently from 102d015 to c5c5ec5 Compare November 22, 2017 13:04
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

Successfully merging this pull request may close these issues.

2 participants