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

Adds sbt.io.syntax #3

Merged
merged 3 commits into from
Sep 12, 2015
Merged

Adds sbt.io.syntax #3

merged 3 commits into from
Sep 12, 2015

Conversation

eed3si9n
Copy link
Member

@eed3si9n eed3si9n commented Jun 8, 2015

The idea is to introduce a single import that can be used as a DSL.

  • For the modular usage people can initially use import sbt.io.syntax._
scala> import sbt.io.syntax._
import sbt.io.syntax._

scala> file(".") glob "*.md"
res0: sbt.io.PathFinder = 

   ./Migration.md

scala> file(".") globRecursive "*.scala"
res1: sbt.io.PathFinder = 

   ./io/src/main/scala/sbt/internal/io/Alternative.scala
   ./io/src/main/scala/sbt/internal/io/DeferredWriter.scala
   ...

/review @jsuereth, @dwijnand

@dwijnand
Copy link
Member

dwijnand commented Jun 8, 2015

LGTM. I like this pattern, and I've been exploring it a lot myself, but I'm still not sure if there's any fall out from it.

object RichURI {
object RichURI extends URIExtra

trait URIExtra {
Copy link
Member

Choose a reason for hiding this comment

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

seriously? Why the trait?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm being consistent with sbt's use of trait in other places like ProjectExtra - https://github.com/sbt/sbt/blob/0.13.8/main/src/main/scala/sbt/Project.scala

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if that's a good style to mimic or not. I'd hate to expose a public trait without a good reason first.

@jsuereth
Copy link
Member

To sum up, I like the syntax object/can buy the reasoning. I don't buy the URIExtra logic, nor do I like how it is for Project in sbt all that much.

@eed3si9n
Copy link
Member Author

eed3si9n commented Sep 2, 2015

@jsuereth @dwijnand We should come up with a systematic style of how we enrich classes in the coding guideline sbt/website#189.

The idea is to introduce a single import that can be used as a DSL.

* For the modular usage people can initially use `import
sbt.io.syntax._`
* IOSyntax can eventually get mixed into `package object sbt`.
@eed3si9n
Copy link
Member Author

eed3si9n commented Sep 5, 2015

Updated IO's syntax to be compliant to the coding guideline.

}

import java.io.File
sealed abstract class IOSyntax0 extends IOSyntax1 {
Copy link
Member

Choose a reason for hiding this comment

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

this is a complication I hadn't thought about. We may need to duplicate these hierarchy levels as well in the sbt package object later....

@jsuereth
Copy link
Member

jsuereth commented Sep 5, 2015

This does look a lot better to me. I would be curious how it affects the sbt main package...

I'm ok merging now. Little worried about implicit priorities...

eed3si9n added a commit that referenced this pull request Sep 12, 2015
@eed3si9n eed3si9n merged commit 7a2fd8b into master Sep 12, 2015
@eed3si9n eed3si9n deleted the wip/syntax branch September 12, 2015 07:14
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.

3 participants