Skip to content

Conversation

kijuky
Copy link
Contributor

@kijuky kijuky commented Oct 18, 2021

Apply asciiGraphWidth setting to dependencyTree tasks for fixing #5962.

Implements:

  • Ignore terminal width. ( This change may not be acceptable. )
  • render for renderingTaskSettings set null. ( Then set the correct value. )

Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Overall I think the fix looks good, but have a minor suggestion for improvement.

Comment on lines 83 to 89
) ++ renderingTaskSettings(dependencyTree, rendering.AsciiTree.asciiTree _)
) ++ {
renderingTaskSettings(dependencyTree, null) :+ {
dependencyTree / asString := {
rendering.AsciiTree.asciiTree(dependencyTreeModuleGraph0.value, asciiGraphWidth.value)
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a bit confusing. It might be better to make an overload of renderingTaskSettings that only takes key: TaskKey[Unit] than passing null here, and implement with the correct dependencyTree / asString in there. Is that possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to pass null either.

Overload! This seems to be the solution!

Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Thanks!

@eed3si9n eed3si9n merged commit bc95ab2 into sbt:develop Oct 22, 2021
@kijuky kijuky deleted the apply-asciigraphwidth-to-dependencytree branch October 22, 2021 04:21
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