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

draft: fix: stop showing output tab in unnecessary circumstances #4337

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

codythomaszeitler
Copy link

What does this PR do?

This PR does the following:
Stops showing the output tab in the following scenarios:
Deploy/Retrieve
Running of tests

I chose these commands for the following reason:
They are ran all of the time.
Everything you would want to know about the output of the command can already be found in the "Problems" tab in a human readable format.

The LibraryCommandletExecutor now defaults to not showing the output tab when the command is ran. The implementer must state explicitly that the output tab should show on run. I believe this is fair, since the project should be aiming for a more intuitive/readable format than just a large format of text from the CLI.

What issues does this PR fix or reference?

#3644

Functionality Before

Whenever a test or deploy is ran, it would swap to the output tab in vscode.

Functionality After

Whenever a test or deploy is ran, it now does not swap to the output tab in vscode.

Notes: I still need to ensure that I have added/fixed any tests that are failing because of these changes. I just wanted to make sure that I am the right path before committing to any solution. I cannot tell how much better this would make my developer experience if just these two changes were to go through (deploy and run tests not showing output anymore)

@randi274
Copy link
Contributor

randi274 commented Aug 2, 2022

Thanks for the contribution @codythomaszeitler - tagging our PM @AnanyaJha on this one to review and see if this is something she'd like us to get in with its current state. We actually will be shipping a new change this week to notifications that you can check out at #4318 (which won't make the extensions less noisy, but more focused at least!).

@codythomaszeitler codythomaszeitler force-pushed the codythomaszeitler/dont-show-output-channel-default branch from d2b82a3 to b2da3c8 Compare August 4, 2022 04:00
@randi274 randi274 added the type:community-contrib Community Contribution Pull Requests label Oct 4, 2022
@git2gus
Copy link

git2gus bot commented Oct 4, 2022

This issue has been linked to a new work item: W-11853975

@AnanyaJha
Copy link
Collaborator

AnanyaJha commented Nov 10, 2022

Hi @codythomaszeitler - thank you for the contribution! @randi274 The overall goal of the request makes sense to me, but there's a few things I'd like to confirm:

  • Problems will still appear in the Problems tab
  • The full CLI result will still appear in the Output tab
  • We do not switch to the Output tab once a deploy/test has finished running. The user can switch to the Output or Problems tab as desired

[I ran into some errors while trying to build this locally, would be lovely if you could share a video or gif of the final functionality]

Although the Problems tab contains info about specific errors, some of our users do rely on the extra info provided by the CLI output. We don't need to auto-switch the tabs for the users, but we should provide them with the same level of info if desired.
Let's talk through the experience when we get closer to merging @randi274 , it'd be good for us to consider this version of the request as well - #4365

@randi274 randi274 requested review from RitamAgrawal and randi274 and removed request for klewis-sfdc and RitamAgrawal April 10, 2023 18:22
@randi274
Copy link
Contributor

Hey @codythomaszeitler - wanted to check in on this PR! We've got some conflicts with our current state of the repo. Ananya is on board with the changes, as long as we haven't impacted the majority of our command output + the CLI output. Given that some time has passed and you originally noted some hesitation about the usefulness of this PR in your own workflow, would you like to gear up on it again and get it over the finish line with updates + test changes, or opt to close this one for now?

@codythomaszeitler
Copy link
Author

Hey @randi274 . I am willing to get this over the finish line.

I do want to clarify some things though.

For my vscode, my output tab does seem to be auto focusing to the output tab when something fails. I am not sure if I am running an old version and that is the reason this is happening, but test failures and deploy failures do seem to focus output for me. (Here is a gif for a deploy failure). (This is in response to @AnanyaJha 's third bullet point above)

Animation

For my developer experience, I would very much appreciate to have the problems tab auto focus in both the deployment failure and in the test failure case. If not that, just not having the IDE focus the output tab would be preferable. My coworkers use intellij with illuminated cloud, and I believe their IDE's auto focus their problems tab when something fails. It normally flows quite naturally for them, since if something has gone wrong, you want to see a clean panel of information denoting why something failed.

To get super specific about how I personally work, it normally goes something like this:

Write failing test with a function/class that most likely does not exist.
Save.
Get a deployment failure (at this point, its focused the output tab)
Use a hotkey to focus the problems tab
Arrow hotkey down to focus the error.
Spacebar to take me to the exact location of where the error was.
Add the function/class
Compile again (passes) (at this point, its focused the output tab)
Run failing test (it refocuses the output tab)
Use a hotkey to focus the problems tab
Arrow hotkey down to focus the error.
Spacebar to take me to the exact location of where the error was.
Fix failing test.

Rinse and repeat.

Just having to refocus the problems tab gets disorienting, since vscode is swapping windows constantly at the bottom of my screen. I would love to have a toggle that allow me to tell the extension to either 1) focus on the problems view if a deployment / test failure has occurred or 2) do not focus the output tab on all sfdx commands (which I believe it is doing).

The changes in this PR should not affect any information that is currently being presented in the output tab.

Let me know if you guys need any other information from me!

@randi274
Copy link
Contributor

randi274 commented May 2, 2023

That's a great overview of your process @codythomaszeitler! Would you be interested in starting a discussion to see if others follow a similar process, and there might be some broader improvements we can make to the notification experience, such as adding a setting to either focus only on failures, or turn off all notifications?

I'd say for the 3rd bullet point (autofocusing on errors), as long as this PR doesn't change what's currently happening, we wouldn't worry about it for merging. To finish this one out, we'd need you to get the latest changes from develop into your branch, and take a stab at writing some unit tests to be code complete. 👍

@codythomaszeitler
Copy link
Author

Hey @randi274 I will start a discussion and also get on those unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants