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.

Sign up for GitHub

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

Width and height override modifiers #389

Merged
merged 12 commits into from
Oct 17, 2020
Merged

Width and height override modifiers #389

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a89d98e
width and height override modifiers
keynmol Sep 18, 2020
cbed9cd
ah yes formatting, I do that
keynmol Sep 18, 2020
1d04b8f
Fix width/height override, manually break up string, hide new functio…
keynmol Sep 29, 2020
bf7e605
Tests and change the docs slightly
keynmol Sep 29, 2020
02360d1
Attempt to please Scalafmt manually
keynmol Sep 29, 2020
d6e8a98
Apply suggestions from code review
keynmol Sep 30, 2020
c5adcf6
Remove newline breaks inside tokens
keynmol Sep 30, 2020
f70d047
Manually reformat again
keynmol Sep 30, 2020
8af4c3c
Try with scalafmt plugin...
keynmol Sep 30, 2020
eab6e49
Merge branch 'master' into width-height-modifiers
keynmol Sep 30, 2020
28415d2
Revert scalafmt plugin
keynmol Sep 30, 2020
21106b3
PR comments
keynmol Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions docs/modifiers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,36 @@ List("with quotes")
```
````

## `width=`

The `width=` allows you to override max width (deault is 80) of pretty-printed values.


````scala mdoc:mdoc
```scala mdoc:width=30
(0 to 100).mkString(",")
```
```scala mdoc
(0 to 100).mkString(",")
```
````


## `height=`

The `height=` allows you to override max height (default is 50) of pretty-printed values.

````scala mdoc:mdoc
```scala mdoc:height=5
List.fill(15)("hello world!")
```
```scala mdoc
List.fill(15)("hello world!")
```
````



## `compile-only`

The `compile-only` modifier ensures the code example compiles without evaluating
Expand Down
10 changes: 10 additions & 0 deletions mdoc/src/main/scala-2/mdoc/internal/markdown/Modifier.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ sealed abstract class Modifier(val mods: Set[Mod]) {
def isResetObject: Boolean = mods(ResetObject)
def isNest: Boolean = mods(Nest)

def widthOverride: Option[Int] =
mods.collectFirst { case Width(value) =>
value
}

def heightOverride: Option[Int] =
mods.collectFirst { case Height(value) =>
value
}

def isToString: Boolean = mods(ToString)
}
object Modifier {
Expand Down
18 changes: 18 additions & 0 deletions mdoc/src/main/scala-2/mdoc/internal/markdown/Renderer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ object Renderer {
def appendMultiline(sb: PrintStream, string: String): Unit = {
appendMultiline(sb, string, string.length)
}

def appendMultiline(sb: PrintStream, string: String, N: Int): Unit = {
var i = 0
while (i < N) {
Expand All @@ -92,6 +93,23 @@ object Renderer {
}
}

def appendMultilineMaxWidth(sb: PrintStream, string: String, maxLineLength: Int): Unit = {
var i = 0
var lineLength = 0

while (i < string.length()) {
string.charAt(i) match {
case '\n' =>
sb.append("\n// "); lineLength = 0
case ch if lineLength == maxLineLength =>
sb.append(ch); sb.append("\n// "); lineLength = 0
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 a bit skeptical about this line wrapping as it breaks the output at arbitrary offsets. I think there is a risk you line wrap right before a newline like this

List(
  "aaaaa
",
  "bbb"
)

I feel like it would be better to make this line behavior opt-in with an additional modifier mdoc:wrap-width=anywhere where the default value is something like wrap-width=tokens or wrap-width=words. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I can imagine there are cases where users would like to set a smaller width while keeping the current line wrapping behavior (split at tokens/words)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking about it, yeah. Width is the problematic one as in the terminal you would expect the behaviour you showed in your example, but in markdown it might look weird.

I need to look closer at what pprint exposes (token-wise) as it indeed would be nicer to break on tokens only, but being able to set anywhere is even better to break long strings (only situation where I can imagine this being useful)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think pprint offers line breaking beyond tokens. I don't mind exposing the ability to break inside tokens but I think it should be an explicit choice from the user

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had a complete brain fart moment :) I struggled to find an example of demonstrating that pprint actually does wrap at given width, around tokens.


```scala mdoc:width=50
List.fill(10)(List(1,2,3,4,5,6,7,8,9,10))
```

```scala
List.fill(10)(List(1,2,3,4,5,6,7,8,9,10))
// res0: List[List[Int]] = List(
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10),
//   List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
// )
```




```scala mdoc:width=20
List.fill(10)(List(1,2,3,4,5,6,7,8,9,10))
```

```scala
List.fill(10)(List(1,2,3,4,5,6,7,8,9,10))
// res1: List[List[Int]] = List(
//   List(
//     1,
//     2,
//     3,
//     4,
//     5,
//     6,
//     7,
//     8,
//     9,
//     10
//   ),
//   List(
//     1,
//     2,
//     3,
//     4,
//     5,
//     6,
//     7,
//     8,
//     9,
//     10
//   ),
//   List(
//     1,
//     2,
//     3,
//     4,
//     5,
//     6,
//     7,
//     8,
//     9,
//     10
//   ),
//   List(
//     1,
//     2,
//     3,
//     4,
//     5,
//     6,
//     7,
//     8,
//     9,
//     10
//   ),
// ...
```

which shows that it works (and I don't need to do manual breaking).

For my usage I don't actually need this "break anywhere" behaviour because it's unpredictable and can lead to bad rendering.

So unless you explicitly want to keep it in (guarded by a custom modifier), I will remove this function altogether and modify the tests - how does that sound?

case ch => sb.append(ch); lineLength += 1
}

i += 1
}
}

def appendFreshMultiline(sb: PrintStream, string: String): Unit = {
val N = string.length - (if (string.endsWith("\n")) 1 else 0)
sb.append("// ")
Expand Down
14 changes: 11 additions & 3 deletions mdoc/src/main/scala-2/mdoc/internal/markdown/ReplVariablePrinter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,24 @@ class ReplVariablePrinter(
if (binder.isToString) {
Renderer.appendMultiline(sb, binder.runtimeValue.toString)
} else {
val heightOverride = binder.mods.heightOverride
val widthOverride = binder.mods.widthOverride

val lines = pprint.PPrinter.BlackWhite.tokenize(
binder.runtimeValue,
width = width,
height = height,
width = widthOverride.getOrElse(width),
height = heightOverride.getOrElse(height),
indent = 2,
initialOffset = baos.size()
)
lines.foreach { lineStr =>
val line = lineStr.plainText
Renderer.appendMultiline(sb, line)
widthOverride match {
case None =>
Renderer.appendMultiline(sb, line)
case Some(maxWidth) =>
Renderer.appendMultilineMaxWidth(sb, line, maxWidth)
}
}
}
baos.toString()
Expand Down
23 changes: 21 additions & 2 deletions mdoc/src/main/scala/mdoc/internal/markdown/Mod.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package mdoc.internal.markdown

import scala.util.Try

sealed abstract class Mod extends Product with Serializable
object Mod {
case object Fail extends Mod
Expand All @@ -23,7 +25,15 @@ object Mod {
}
case object Nest extends Mod

def all: List[Mod] =
case class Width(value: Int) extends Mod {
override def toString: String = s"width=$value"
}

case class Height(value: Int) extends Mod {
override def toString: String = s"height=$value"
}

def static: List[Mod] =
List(
Passthrough,
Invisible,
Expand All @@ -38,7 +48,16 @@ object Mod {
ToString,
Nest
)

def parametric: List[String => Option[Mod]] =
Copy link
Member

Choose a reason for hiding this comment

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

nit: I feel like the following solution is a bit nicer

import scala.util.Try
sealed abstract class Mod extends Product with Serializable
case object Passthrough extends Mod
val static = List[Mod](Passthrough)
case class Width(n: Int) extends Mod
case class Heigth(n: Int) extends Mod
val ToWidth = "width=(\\d+)".r
val ToHeight = "height=(\\d+)".r
object ToInt {
  def unapply(s: String): Option[Int] = Try(s.toInt).toOption
}

List("width=24", "height=42", "width=abc").map {
  case ToWidth(ToInt(n)) => Some(Width(n))
  case ToHeight(ToInt(n)) => Some(Heigth(n))
  case _ => None
} // List(Some(Width(24)), Some(Height(42)), None)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice.

I've changed it very slightly, how does 21106b3 look?

List(
s => Try(s.replace("width=", "").toInt).toOption.map(Width),
s => Try(s.replace("height=", "").toInt).toOption.map(Height)
)

def unapply(string: String): Option[Mod] = {
all.find(_.toString.equalsIgnoreCase(string))
static.find(_.toString.equalsIgnoreCase(string)) orElse parametric
Copy link
Member

Choose a reason for hiding this comment

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

nit: let's avoid non-symbolic infix operators

Suggested change
static.find(_.toString.equalsIgnoreCase(string)) orElse parametric
static.find(_.toString.equalsIgnoreCase(string)).orElse {
parametric.flatMap(check => check(string)).headOption
}

.flatMap(check => check(string))
.headOption
}
}
94 changes: 94 additions & 0 deletions tests/unit/src/test/scala/tests/markdown/WidthHeightModifierSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package tests.markdown

class WidthHeightModifierSuite extends BaseMarkdownSuite {

check(
"no-width-override",
"""
|```scala mdoc
|(0 to 100).mkString(",")
|```
""".stripMargin,
"""
|```scala
|(0 to 100).mkString(",")
|// res0: String = "0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100"
|```
""".stripMargin
)

check(
"width-override",
"""
|```scala mdoc:width=30
|(0 to 100).mkString(",")
|```
""".stripMargin,
"""
|```scala
|(0 to 100).mkString(",")
|// res0: String = "0,1,2,3,4,5,6,7,8,9,10,11,12,1
|// 3,14,15,16,17,18,19,20,21,22,23
|// ,24,25,26,27,28,29,30,31,32,33,
|// 34,35,36,37,38,39,40,41,42,43,4
|// 4,45,46,47,48,49,50,51,52,53,54
|// ,55,56,57,58,59,60,61,62,63,64,
|// 65,66,67,68,69,70,71,72,73,74,7
|// 5,76,77,78,79,80,81,82,83,84,85
|// ,86,87,88,89,90,91,92,93,94,95,
|// 96,97,98,99,100"
|```
""".stripMargin
)

check(
"height-override",
"""
|```scala mdoc:height=5
|List.fill(15)("hello world!")
|```
""".stripMargin,
"""
|```scala
|List.fill(15)("hello world!")
|// res0: List[String] = List(
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// ...
|```
""".stripMargin
)

check(
"no-height-override",
"""
|```scala mdoc
|List.fill(15)("hello world!")
|```
""".stripMargin,
"""
|```scala
|List.fill(15)("hello world!")
|// res0: List[String] = List(
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!",
|// "hello world!"
|// )
|```
""".stripMargin
)

}