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

Do not steal the focus when showing the channels #2828

Merged
merged 2 commits into from
Jan 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Fixed finding references to operator overloads _(Contributed by [@SirIntruder](https://github.com/SirIntruder))_ (PR: [omnisharp-roslyn#1371](https://github.com/OmniSharp/omnisharp-roslyn/pull/1371))
* Improved handling of files moving on disk (PR: [omnisharp-roslyn#1368](https://github.com/OmniSharp/omnisharp-roslyn/pull/1368))
* Improved detection of MSBuild when multiple instances are available _(Contributed by [@johnnyasantoss ](https://github.com/johnnyasantoss))_ (PR: [omnisharp-roslyn#1349](https://github.com/OmniSharp/omnisharp-roslyn/pull/1349))
* Fixed a bug where the "OmniSharp" and "C# log" would steal the editor focus and hinder the user's development flow.(PR: [#2828](https://github.com/OmniSharp/omnisharp-vscode/pull/2828))

#### Debugger
* Added support for set next statement. Set next statement is a feature that have been available for a long time in full Visual Studio, and this brings the feature to Visual Studio Code. This feature allows developers to change what code is executed in your program. For example, you can move the instruction pointer back to re-execute a function that you just ran so you can debug into it, or you can skip over some code that you don't want to execute. To use this feature, move your cursor to the next statement you would like to execute next, and either open the editor context menu and invoke 'Set Next Statement (.NET)', or use the keyboard shortcut of <kbd>Ctrl+Shift+F10</kbd> ([#1753](https://github.com/OmniSharp/omnisharp-vscode/issues/1753))
Expand Down
4 changes: 1 addition & 3 deletions src/observers/CsharpChannelObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,11 @@ export class CsharpChannelObserver extends BaseChannelObserver {
switch (event.constructor.name) {
case PackageInstallStart.name:
case IntegrityCheckFailure.name:
this.showChannel(true);
break;
case InstallationFailure.name:
case DebuggerNotInstalledFailure.name:
case DebuggerPrerequisiteFailure.name:
case ProjectJsonDeprecatedWarning.name:
this.showChannel();
this.showChannel(true);
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/observers/DotnetChannelObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class DotNetChannelObserver extends BaseChannelObserver {
switch (event.constructor.name) {
case CommandDotNetRestoreStart.name:
this.clearChannel();
this.showChannel();
this.showChannel(true);
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/observers/DotnetTestChannelObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default class DotnetTestChannelObserver extends BaseChannelObserver {
case DotNetTestsInClassRunStart.name:
case DotNetTestDebugStart.name:
case DotNetTestsInClassDebugStart.name:
this.showChannel();
this.showChannel(true);
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/observers/OmnisharpChannelObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class OmnisharpChannelObserver extends BaseChannelObserver {
case ShowOmniSharpChannel.name:
case OmnisharpFailure.name:
case OmnisharpServerOnStdErr.name:
this.showChannel();
this.showChannel(true);
break;
case OmnisharpRestart.name:
this.clearChannel();
Expand Down
19 changes: 2 additions & 17 deletions test/unitTests/logging/CsharpChannelObserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,11 @@ suite("CsharpChannelObserver", () => {
new InstallationFailure("someStage", "someError"),
new DebuggerNotInstalledFailure(),
new DebuggerPrerequisiteFailure("some failure"),
new ProjectJsonDeprecatedWarning()
].forEach((event: BaseEvent) => {
test(`${event.constructor.name}: Channel is shown`, () => {
let hasShown = false;

let observer = new CsharpChannelObserver({
...getNullChannel(),
show: () => { hasShown = true; }
});

observer.post(event);
expect(hasShown).to.be.true;
});
});

[
new ProjectJsonDeprecatedWarning(),
new IntegrityCheckFailure("", "", true),
new PackageInstallStart()
].forEach((event: BaseEvent) => {
test(`${event.constructor.name}: Channel is shown and preserveFocus is set to true`, () => {
test(`${event.constructor.name}: Channel is shown and preserve focus is set to true`, () => {
let hasShown = false;
let preserveFocus = false;
let observer = new CsharpChannelObserver({
Expand Down
9 changes: 7 additions & 2 deletions test/unitTests/logging/DotnetTestChannelObserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ import DotnetTestChannelObserver from '../../../src/observers/DotnetTestChannelO

suite("DotnetTestChannelObserver", () => {
let hasShown: boolean;
let preserveFocus: boolean;

let observer = new DotnetTestChannelObserver({
...getNullChannel(),
show: () => { hasShown = true; }
show: (preserve) => {
hasShown = true;
preserveFocus = preserve;
}
});

setup(() => {
Expand All @@ -27,10 +31,11 @@ suite("DotnetTestChannelObserver", () => {
new DotNetTestDebugStart("foo"),
new DotNetTestsInClassDebugStart("someclass")
].forEach((event: BaseEvent) => {
test(`${event.constructor.name}: Channel is shown`, () => {
test(`${event.constructor.name}: Channel is shown and preserve focus is set to true`, () => {
expect(hasShown).to.be.false;
observer.post(event);
expect(hasShown).to.be.true;
expect(preserveFocus).to.be.true;
});
});
});
10 changes: 8 additions & 2 deletions test/unitTests/logging/OmnisharpChannelObserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,19 @@ suite("OmnisharpChannelObserver", () => {

let hasShown: boolean;
let hasCleared: boolean;
let preserveFocus: boolean;
let observer: OmnisharpChannelObserver;

setup(() => {
hasShown = false;
hasCleared = false;
preserveFocus = false;
observer = new OmnisharpChannelObserver({
...getNullChannel(),
show: () => { hasShown = true; },
show: (preserve) => {
Copy link

Choose a reason for hiding this comment

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

Do we ever call this with false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a case where we want to steal the focus

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As of now I couldnt think of any particular case hence right now we are using true everywhere

Copy link

Choose a reason for hiding this comment

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

Why not make it even easier and remove the paramter/always pass true in this method?

hasShown = true;
preserveFocus = preserve;
},
clear: () => { hasCleared = true; }
});
});
Expand All @@ -28,10 +33,11 @@ suite("OmnisharpChannelObserver", () => {
new ShowOmniSharpChannel(),
new OmnisharpServerOnStdErr("std err")
].forEach((event: BaseEvent) => {
test(`${event.constructor.name}: Channel is shown`, () => {
test(`${event.constructor.name}: Channel is shown and preserveFocus is set to true`, () => {
expect(hasShown).to.be.false;
observer.post(event);
expect(hasShown).to.be.true;
expect(preserveFocus).to.be.true;
});
});

Expand Down