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.

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

RUN: show build error balloon over proper tool window #7792

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

Undin
Copy link
Member

@Undin Undin commented Sep 4, 2021

Previously, build manager showed build error balloon over Messages tool window tab for some reason.
But the plugin doesn't interact with this tool window at all and prints all build output to Build tool window.
Moreover, in most cases Messages tool window doesn't have any active tab and a user can't even open it. It's rather confusing

After these changes, the plugin will show build error balloon over Build tool window tab to be consistent with build output and not confuse users

Before After
image image

changelog: Show build error balloon over Build tool window instead of Messages one

Previously, build manager showed build error balloon over `Messages` tool window tab for some reason.
But the plugin doesn't interact with this tool window at all and prints all build output to `Build` tool window.
Moreover, in most cases `Messages` tool window doesn't have any active tab and a user can't even open it. It's rather confusing

After these changes the plugin will show build error balloon over `Build` tool window tab to be consistent with build output and not confuse users
@Undin Undin added the fix Pull requests that fix some bug(s) label Sep 4, 2021
@@ -270,10 +268,9 @@ object CargoBuildManager {
notification.notify(project)

if (messageType === MessageType.ERROR) {
MessageView.SERVICE.getInstance(project) // register ToolWindowId.MESSAGES_WINDOW
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure that we don't need call BuildContentManager.getInstance(project).getOrCreateToolWindow() here instead.
@mchernyavsky why did we call it here before?

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 not sure. It is copypasted from CidrBuild.showBuildNotification :)

@Undin
Copy link
Member Author

Undin commented Sep 4, 2021

For QA: I've changed some code with tool window initialization, so it would be cool to check not only balloon location but also that Build tool window is created properly by actions that trigger project builds even if tool window was closed before

Copy link
Member

@mchernyavsky mchernyavsky left a comment

Choose a reason for hiding this comment

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

LGTM

@Undin
Copy link
Member Author

Undin commented Oct 1, 2021

bors r=mchernyavsky

@bors
Copy link
Contributor

bors bot commented Oct 1, 2021

Build succeeded:

@bors bors bot merged commit 91607c0 into master Oct 1, 2021
@bors bors bot deleted the undin/show-build-balloon-properly branch October 1, 2021 18:42
@github-actions github-actions bot added this to the v157 milestone Oct 1, 2021
@neonaot neonaot self-assigned this Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests that fix some bug(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants