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

Add push option in createOutputChannel API #58116

Merged
merged 4 commits into from
Sep 21, 2018
Merged

Conversation

sandy081
Copy link
Member

@sandy081 sandy081 commented Sep 6, 2018

After implementing Output channels to log into files directly from extension host and let the renderer listen to file changes, a delay got introduced between adding the data and it is shown in the UI. It might not be expected for some output channels who intend to show real time data. This PR introduces an option push that extension authors can use while creating an output channel to decide whether to push the data and show in the UI immediately.

#57618

@sandy081 sandy081 added output Output channel system issues api-finalization labels Sep 6, 2018
@sandy081 sandy081 added this to the September 2018 milestone Sep 6, 2018
@sandy081 sandy081 self-assigned this Sep 6, 2018
@jrieken jrieken added the api label Sep 7, 2018
@jrieken
Copy link
Member

jrieken commented Sep 13, 2018

Alternative ideas for the parameter name are force, live, noBuffer, inMemory, or something that expresses this is for logging.

@sandy081
Copy link
Member Author

I liked live option out of all as it also goes well with what we describes about the output channel. Also how about noDelay?

Creates a new [output channel](#OutputChannel) with the given name.
		 * By default there will be a delay in showing the appended data in the UI.
		 * Pass `true` for `live` argument to show the data immediately.
		 *
		 * *NOTE* Default output channels help in improving the performance of VS Code

@sandy081
Copy link
Member Author

@jrieken Any updates on this. I would like to change the second parameter to options object so that it can be extensible in future

options: {noDelay: boolean}

@jrieken
Copy link
Member

jrieken commented Sep 20, 2018

Not a fan of noDelay because that sounds like I would never want a delay. We should pick a word that expresses this being hard on vscode, like force or noBuffer

@sandy081
Copy link
Member Author

I think noBuffer also has similar sense of noDelay. force might be good one to use. Updated the PR.

*/
export function createOutputChannel(name: string): OutputChannel;
export function createOutputChannel(name: string, options?: { force?: boolean }): OutputChannel;
Copy link
Member

Choose a reason for hiding this comment

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

why an object? are there plans for more options?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, there is a request to set a lang id for channels. If so it has to be set while creating

@@ -6221,11 +6221,16 @@ declare module 'vscode' {
export function createInputBox(): InputBox;

/**
* Create a new [output channel](#OutputChannel) with the given name.
* Creates a new [output channel](#OutputChannel) with the given name.
* By default the UI will not show the appended data immediately.
Copy link
Member

Choose a reason for hiding this comment

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

Hm, we should sell this better... How about: "Output channels use a separate transport mechanism when appending data. This helps to reduce CPU load on the UI process but also means that the output channel UI updates with a small delay. Use the force-flag to enforce immediate updates at the cost of higher CPU usage"

Copy link
Member Author

Choose a reason for hiding this comment

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

I like it. You have better marketing skills 👍 😄

@sandy081
Copy link
Member Author

Updated

Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

🚢 it

@sandy081 sandy081 merged commit ab93c7e into master Sep 21, 2018
@sandy081 sandy081 deleted the sandy081/outputChannel branch September 21, 2018 08:16
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization output Output channel system issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants