Skip to content

Rewrote implicits section of the tour #746 #998

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

Closed
wants to merge 3 commits into from
Closed

Rewrote implicits section of the tour #746 #998

wants to merge 3 commits into from

Conversation

mghildiy
Copy link
Contributor

No description provided.

@mghildiy
Copy link
Contributor Author

I added code snippets for explaining implicit class. Build fails for that code:

package prac.implicits
 
object StringNuggets {
  implicit class StringImprovements(val s: String) {
    def increment = s.map(c => (c + 1).toChar)
  }
}

package prac.implicits

import prac.implicits.StringNuggets._

object ImplicitDemo {

  def main(args: Array[String]): Unit = {
    println("Einstein".increment)
  }
}

How to work around it?

@SethTisue
Copy link
Member

it appears to me that tut doesn't support package declarations. are they necessary here? if the example can do without them, you can remove them. if they are necessary, you can disable tut by removing the tut after the triple backquote at the top of the code snippet in question

@mghildiy
Copy link
Contributor Author

Do I need to remove older implicit files(implicit-parameters.md,implicit-conversions.md) from language specific directories(_ba\tour,_es\tour,_ko\tour etc) too?It seems jekyll build tries to link these files to the older implcit files I have deleted from _tour, and hence fails.

@mghildiy
Copy link
Contributor Author

Build passed. Ready for review.

@mghildiy
Copy link
Contributor Author

Can anybody review it?

@jvican
Copy link
Member

jvican commented Jan 19, 2018

Sorry for the delay @mghildiy, I'll have a look at these over this w-e.

@mghildiy
Copy link
Contributor Author

mghildiy commented Jan 19, 2018 via email

@NthPortal
Copy link
Contributor

I'd like to note (as I did in the original PR) that the Swing example doesn't not require an implicit conversion to work in 2.12, because of support for Java 8 lambdas.

Unfortunately, I don't have any ideas for an example to use instead.

@jvican
Copy link
Member

jvican commented Jan 19, 2018

There are many issues you could help with in https://github.com/scala/docs.scala-lang/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc 😄 Pick the one you like the most. If you prefer another style of issue, lemme know.

In particular, I think that working on a robust way to have the spec in both PDFs and ebook format would be great, see this ticket: scala/bug#10218. I believe that this can have a high impact, making the spec easier to read in different formats is always a win.

@mghildiy
Copy link
Contributor Author

mghildiy commented Jan 20, 2018 via email

@jvican
Copy link
Member

jvican commented Jan 21, 2018

@mghildiy Let's chat privately, can you ping me in Gitter?

@SethTisue
Copy link
Member

SethTisue commented Jan 22, 2018

two things that would need fixing before this could be mergeable:

  • the changes to the headers don't match the current standard. (Travis drafted his changes before the big site overhaul, so you should base the headers on what the other tour sections have, not on what's in Travis's draft.)
  • you shouldn't put very long /** comments */ inside code blocks, because they won't line wrap. it's better for these texts to be outside, in the body text, and keep any comments inside very brief.

@mghildiy
Copy link
Contributor Author

mghildiy commented Jan 23, 2018

Code blocks means 'tut' sections. Can you please mention here the line no with long comments?

@SethTisue
Copy link
Member

an example very long comment:

/** To show how implicit parameters work, we first define monoids for strings and integers. The implicit keyword indicates that the corresponding object can be used implicitly, within this scope, as a parameter of a function marked implicit. */

there are a number of similar such comments in the diffs.

@mghildiy
Copy link
Contributor Author

Hi,

What should be the order of file navigation?Previous page for Implicit-conversions is 'explicitly-typed-self-references'(which though doesn't exist, and I guess it should be 'self-types'). And implcits too has same previous page.

@SethTisue
Copy link
Member

What should be the order of file navigation

the existing order at http://docs.scala-lang.org/tour/tour-of-scala.html seems fine to me

I just noticed that if this PR were merged, we'd have three sections on implicits. did you mean to remove some of them...?

@mghildiy
Copy link
Contributor Author

mghildiy commented Jan 26, 2018

implicit parameters and implicit conversions are now part of implicits.md. So I would remove these two older files.I have also pinged Travis for some inputs on this change.

@mghildiy mghildiy closed this Jan 26, 2018
@SethTisue
Copy link
Member

new PR is #1003

@mghildiy
Copy link
Contributor Author

mghildiy commented Feb 3, 2018

@NthPortal , do you have a good example in mind for the section you want to be modified?

@NthPortal
Copy link
Contributor

@mghildiy Unfortunately I don't, sorry. My only thought is that there's probably a nice implicit conversion in the standard library that can be used; perhaps something in Predef (something like Array[A] => Seq[A], but less complicated).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants