Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

# for free to join this conversation on GitHub. Already have an account? # 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