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

Jump to bottom

add detailed error config (#71) #142

Merged
merged 3 commits into from
Jul 8, 2021
Merged

add detailed error config (#71) #142

merged 3 commits into from
Jul 8, 2021

Conversation

muhammedbehram
Copy link
Contributor

Created a setting key as mentioned in issue #71 and passed it into tasks.

poslegm
poslegm requested changes Jul 3, 2021
View reviewed changes
Copy link
Collaborator

@poslegm poslegm left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM with small remarks

plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala Outdated
@@ -323,14 +358,15 @@ object ScalafmtPlugin extends AutoPlugin {
}

private def scalafmtTask =
Def.task {
Def.task { //TODO implement from here
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this comment means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a reminder for myself. Sorry that I failed to check it before sending the commit. I'll remove it and send it with other changes.

plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala Outdated
@@ -100,11 +102,12 @@ object ScalafmtPlugin extends AutoPlugin {
config: Path,
log: Logger,
writer: OutputStreamWriter,
resolvers: Seq[Resolver]
resolvers: Seq[Resolver],
isDetailedError: Boolean
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
isDetailedError: Boolean
detailedErrorEnabled: Boolean

more understandable name

plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala Outdated
@@ -62,6 +62,8 @@ object ScalafmtPlugin extends AutoPlugin {
"Execute the scalafmtCheck task for all configurations in which it is enabled. " +
"(By default this means the Compile and Test configurations.)"
)
val scalafmtDetailedError =
settingKey[Boolean]("Log detailed error with stack trace, off by default")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about English grammar :)

May be Enables logging of detailed errors with stacktraces, disabled by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sounds better :)

@poslegm
Copy link
Collaborator

poslegm commented Jul 4, 2021

Compilation errors:

[error] /home/runner/work/sbt-scalafmt/sbt-scalafmt/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala:432:17: The key `scalafmtTask` is not being invoked inside the task definition.
[error] 
[error] problem:  Keys missing `.value` are not initialized and their dependency is not registered.
[error] solution: Replace `scalafmtTask` by `scalafmtTask.value` or remove it if unused.
[error]     
[error]     scalafmt := scalafmtTask,

You can test plugin locally with sbt publishLocal or run unit tests with sbt plugin/scripted

@muhammedbehram
Copy link
Contributor Author

muhammedbehram commented Jul 5, 2021 •
edited
Loading

You can test plugin locally with sbt publishLocal or run unit tests with sbt plugin/scripted

After I fixed the complication issue,
I tried sbt publishLocal both on master(unedited) branch and add-detailed-error-config branch. It failed with the same error.
[error] Cause of test exception: {line 83} Command succeeded but failure was expected
This issue seems to be a local one but I thought it would be better for me to report this before sending the fix commit for the compilation error.

@poslegm
Copy link
Collaborator

poslegm commented Jul 5, 2021

This issue seems to be a local one but I thought it would be better for me to report this before sending the fix commit for the compilation error.

It's strange. Looks really like problem in local environment because I built plugin successfully just now.

@muhammedbehram
Copy link
Contributor Author

It's strange. Looks really like problem in local environment because I built plugin successfully just now.

I couldn't find a solution yet. Still committed changes. Would you like me to squash commits?

@poslegm poslegm merged commit 48995db into scalameta:master Jul 8, 2021
@poslegm
Copy link
Collaborator

poslegm commented Jul 8, 2021

I tested it locally, everything is fine 👍🏻

@poslegm
Copy link
Collaborator

poslegm commented Jul 8, 2021

You can add flag description here after v2.4.3 will be released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poslegm poslegm poslegm requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@muhammedbehram @poslegm