Add preference clear output channel#4318
Conversation
| } | ||
|
|
||
| public streamCommandStartStop(execution: CommandExecution) { | ||
| if (SfdxSettingsService.getEnableClearOutputBeforeEachCommand()) { |
There was a problem hiding this comment.
The channel service is my first choice to attempt to handle this preference in one central location. Unfortunately, the way the channel service is used throughout the app is very inconsistent. This "streamCommandStartStop()" is used by some of the abstract executors ( SfdxCommandletExecutor, LibraryExectutor ) that are exported from salesforcedx-utils-vscode.
| const startTime = process.hrtime(); | ||
| const channelService = new ChannelService(this.outputChannel); | ||
| const telemetryService = TelemetryService.getInstance(); | ||
| if (SfdxSettingsService.getEnableClearOutputBeforeEachCommand()) { |
There was a problem hiding this comment.
The LibraryCommandletExecutor is exported from salesforcedx-utils-vscode and is another place that I found to check and clear the output log.
| @@ -0,0 +1,3 @@ | |||
| export const SETTING_CLEAR_OUTPUT_TAB = 'clearOutputTab'; | |||
There was a problem hiding this comment.
These are now all exported by salesforcedx-utils-vscode and consumed in other modules so that the string 'salesforcedx-vscode-core' is not in 20 places in various modules of the mono-repo.
| } | ||
|
|
||
| public async run(): Promise<void> { | ||
| if (sfdxCoreSettings.getEnableClearOutputBeforeEachCommand()) { |
There was a problem hiding this comment.
Adding this to the run() method here basically adds this feature to all the commands that extend SfdxCommandlet in the salesforcedx-vscode-core extension. I wish there was an elegant single cut-point elsewhere in the salesforcedx-utils-vscode module like this, but there was not.
…ithub.com/forcedotcom/salesforcedx-vscode into dehru/add-preference-clear-output-channel
randi274
left a comment
There was a problem hiding this comment.
Looking great, and look at those lovely unit tests! 😍 Just a few quick qs. Good for me otherwise pending QA
packages/salesforcedx-utils-vscode/src/commands/channelService.ts
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,3 @@ | |||
| export const SETTING_CLEAR_OUTPUT_TAB = 'clearOutputTab'; | |||
| export const SFDX_CORE_CONFIGURATION_NAME = 'salesforcedx-vscode-core'; | |||
There was a problem hiding this comment.
What's your thought here having these two identical strings with different names?
There was a problem hiding this comment.
I decided that since they represent 2 different things, that we should have 2 constants, even though they are the same string.
I wanted anyone changing these in the future to be more aware of what this constant represents.
| * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
| */ | ||
| import * as vscode from 'vscode'; | ||
| import { SfdxSettingsService } from '../settings'; |
There was a problem hiding this comment.
I don't think this needs to be imported
| ); | ||
| const showCLISuccessMsg = vscode.workspace | ||
| .getConfiguration('salesforcedx-vscode-core') | ||
| .getConfiguration(SFDX_CORE_CONFIGURATION_NAME) |
There was a problem hiding this comment.
Since
vscode.workspace
.getConfiguration(SFDX_CORE_CONFIGURATION_NAME)
is called twice, how about changing to:
const coreConfigurationName = vscode.workspace.getConfiguration(SFDX_CORE_CONFIGURATION_NAME);
const showCLISuccessMsg = coreConfigurationName.get<boolean>('show-cli-success-msg', true);
..and then using coreConfigurationName on line 132 & 133.
| getRootWorkspacePath, | ||
| getRootWorkspaceSfdxPath | ||
| } from './workspaces'; | ||
| export { SETTING_CLEAR_OUTPUT_TAB, SFDX_CORE_CONFIGURATION_NAME, SFDX_CORE_EXTENSION_NAME } from './constants'; |
There was a problem hiding this comment.
the imports need to be alphabetized. I'm surprised the linter didn't catch this.
Since this is being imported from ./constants, it should be moved to line 25, before the import from ./context/...
| import { notificationService } from '@salesforce/salesforcedx-utils-vscode/out/src/commands'; | ||
| import { TraceFlags } from '@salesforce/salesforcedx-utils-vscode/out/src/helpers'; | ||
| import * as utils from '@salesforce/salesforcedx-utils-vscode/out/src/index'; | ||
| import { SFDX_CORE_CONFIGURATION_NAME } from '@salesforce/salesforcedx-utils-vscode/out/src/index'; |
There was a problem hiding this comment.
since @salesforce/salesforcedx-utils-vscode/out/src/index is already being imported, can line 15 be removed and on line 54, use utils.SFDX_CORE_CONFIGURATION_NAME?
|
|
||
| public getEnableClearOutputBeforeEachCommand(): boolean { | ||
| return this.getConfigValue(SETTING_CLEAR_OUTPUT_TAB, false); | ||
| } |
There was a problem hiding this comment.
nit pick: please add an extra carriage return after the closing curly brace.
|
QA Notes: Notes:
|
What does this PR do?
This PR adds a user/workspace preference and changes the preference category they display under in the preferences UI. When set, the output channel is cleared between each common command. There are a few commands that do not output to the channel ( Like Create Soql Query ) and those do not clear the channel.
What issues does this PR fix or reference?
#941, @W-10288504@
Functionality Before
Old and New Preference Shown Below
Functionality After
Screen.Recording.2022-07-25.at.4.30.25.PM.mov