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

Turn off "restore" notifications? #193

Closed
ciel opened this issue Apr 15, 2016 · 21 comments
Closed

Turn off "restore" notifications? #193

ciel opened this issue Apr 15, 2016 · 21 comments

Comments

@ciel
Copy link

ciel commented Apr 15, 2016

I love the work you guys have done, and I am thankful that the extension tries to restore my dependencies - but is there a way to ask it to ... well, not?

It's a bit obtrusive to have it keep trying that constantly. Sometimes - actually most times - I need to do a restore when I'm ready to. I've had situations where I have stacks of 10 notifications keep pouring over the top of my screen trying to force me to restore my projects when that's just not what I want to do.

The same with the debugger. Can we set it to be downloaded and installed when we ask it to - instead of it always doing it for every project, automatically, the second it is opened? It makes it very hard to rapidly move between things, copy things out of old projects, etc. It's constantly doing things I'm really not asking it to do.

@DustinCampbell
Copy link
Member

The debugger shouldn't be downloaded for each project. Are you referring to the popup that asks if you want to add tasks.json and launch.json?

@d0pare
Copy link

d0pare commented Apr 15, 2016

This is the original bug OmniSharp/omnisharp-roslyn#504

@ciel
Copy link
Author

ciel commented Apr 16, 2016

Yes, that would be the bug it seems. Sorry, I guess that part was a pre-existing issue. I thought I was just going crazy.

@DustinCampbell
Copy link
Member

No worries. It's something I've heard a number of times. I'll look at improving that experience. I think we can do some things in the C# extension for VS Code to make it better, as well as improving the OmniSharp server itself.

@DustinCampbell
Copy link
Member

Hi @ciel, is this still an issue for you with the latest C# extension?

@ciel
Copy link
Author

ciel commented Aug 27, 2016

Yes. It is such a huge issue that VSC is completely unusable.

On Aug 27, 2016 5:52 AM, "Dustin Campbell" notifications@github.com wrote:

Hi @ciel https://github.com/ciel, is this still an issue for you with
the latest C# extension?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#193 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAavtCtov5r-X5sB1_mITlwHq1AWCLa_ks5qkBbugaJpZM4IIa51
.

@dasMulli
Copy link
Contributor

dasMulli commented Nov 2, 2016

+1 because this becomes pretty annoying when e.g. opening a projects that are meant to be (or just have been) restored with special arguments by a build script (e.g. dotnet/cli).
But then it's pretty handy when working on new / small projects... so being able to configure it would be awesome.
close-all-the-infos

@guardrex
Copy link

guardrex commented Nov 2, 2016

[EDIT] @DustinCampbell There is some discussion on Slack about the auto-restore notification again. Everyone there is in agreement that VS Code should have a setting that allows us to turn off the auto-restore notification. I'm surprised this wouldn't be fairly easy to pull off. Isn't it just a workspace setting (bool) and an if block on the dotnet restore bits?

[EDIT] It's not already done? https://github.com/OmniSharp/omnisharp-roslyn/blob/be0c664fdf1ec652cfc1215f1b4ba07f6583c500/src/OmniSharp.DotNet/DotNetProjectSystem.cs#L109

[EDIT] Rubber duck checking stuff. We can't mod 👉 👉 👉 https://github.com/OmniSharp/omnisharp-roslyn/blob/cdd99aef194660835e61c58f19fb51ab3dabc52e/src/OmniSharp/config.json

So are we talk'in about short-circuiting to avoid the listener here.
https://github.com/OmniSharp/omnisharp-vscode/blob/042ab2ad1dbcc175d89c838aa5990474bd1c39b3/src/omnisharp/server.ts#L166

public onUnresolvedDependencies(listener: (e: protocol.UnresolvedDependenciesMessage) => any, 
        thisArg?: any) {
    return this._addListener(Events.UnresolvedDependencies, listener, thisArg);
}

@DustinCampbell
Copy link
Member

Where in Slack is this discussion happening? On the official OmniSharp Slack? I'd love to be a part of any discussion, or better yet, have the discussion here in the open.

Oh, I agree too. It's just a matter of finding the time to get this feature done at the moment, with other priorities. It's pretty high on my list, but not the highest. I have a couple of thoughts.

  1. I suspect that everyone would like the restore popup quite a bit more if it didn't appear for every single change in the universe, but grouped them together into one prompt. I suspect that would be helpful.
  2. It'd be great if we could be a bit smarter when someone does a normal operation like a git clean -xdf and not offer to restore immediately.

I don't think the change you suggest is the right one. We shouldn't be killing the event based on an option, but rather providing smarter behavior when the event fires.

@guardrex
Copy link

guardrex commented Nov 3, 2016

It was on the aspnetcore Slack, and I mis-described it. It's was more of a "rant" among a couple of devs than a "discussion."

I'm sure it will be great when restore is smarter and better. However, I disagree with your point about not offering a way to completely disable automatic restore.

[EDIT] I mis-typed that --- I mean to say auto-restore notification. I just don't want to be told that a restore is needed. I'll know/trigger on my own would be best for me.

I use my extension, Status Bar Tasks, and when I want a restore, I trigger it manually with one click in the status bar. I can't think of a single situation where I would want an automatic restore. I prefer to be able to configure Omnisharp with a workspace setting that permanently disables it.

On the other hand and as I was saying to @shaunluttin, any of us can just clone the repo and make a version of this that does whatever we like, and I thank you, the Omnisharp team, and MS for making that possible. 👍

Based on your response, it seems like the winds are blowing in the direction of never having a permanent disable option for the notification, so let me ask this in case I decide to clone and hack: Would it be as simple as removing ...

public onUnresolvedDependencies(listener: (e: protocol.UnresolvedDependenciesMessage) => any, 
        thisArg?: any) {
    return this._addListener(Events.UnresolvedDependencies, listener, thisArg);
}

... and the d3 bit at ... https://github.com/OmniSharp/omnisharp-vscode/blob/ffc52290366467417cc7d673e4756821d8e19827/src/features/status.ts#L245-L256

let d3 = server.onUnresolvedDependencies(message => {
    let info = `There are unresolved dependencies from 
        '${vscode.workspace.asRelativePath(message.FileName) }'. Please execute the restore 
        command to continue.`;

    return vscode.window.showInformationMessage(info, 'Restore').then(value => {
        if (value) {
            dotnetRestoreForProject(server, message.FileName);
        }
    });
});

return vscode.Disposable.from(d0, d1, d2, d3);

... from server.ts?

@DustinCampbell
Copy link
Member

Oh, I didn't say that I disagreed with offering a way to disable. I specifically said that I agreed. 😄

On disabling, yes, it's certainly possible to do it in the extension (I'm taking PRs BTW). However, I've been wondering if it's probably better done in OmniSharp itself. After all, why bother registering a bunch of FIleSystemWatchers and firing events that aren't going to get handled.

@shaunluttin
Copy link

shaunluttin commented Nov 3, 2016

@DustinCampbell I am the developer who started the rant about disabling the popup. Here are the things on my wish list:

  1. A way to permanently disable the Info popup, when I decide it no longer provides value.
  2. A keyboard shortcut to close the Info popup, so that I can keep my hands on the keyboard.

Right now, the Info popup significantly interferes with my flow during development.

capture

How can we close that without the mouse? How do we prevent it from arising in the first place?

@DustinCampbell
Copy link
Member

Sounds good. 😄

@shaunluttin
Copy link

shaunluttin commented Nov 3, 2016

Regarding the last @DustinCampbell comment. I deleted the comment to which he replied, which asked about why it is called "auto-restore."

@shaunluttin
Copy link

@DustinCampbell Is there an existing keyboard shortcut to close those Info notifications?

@shaunluttin
Copy link

shaunluttin commented Nov 3, 2016

It is actually almost impossible to develop in Visual Studio Code without a mouse unless we restore first. That Info popup will recur over and over and over...

@DustinCampbell
Copy link
Member

I'm not sure if there's a keyboard shortcut or not. That's just the standard VS Code UI for displaying an information message.

@guardrex
Copy link

guardrex commented Nov 3, 2016

Sorry guys ... lack of paying attention on my part when discussing: It's not auto-restore ... it's auto-notification that a restore is needed. I just don't need to be told that. Sorry again.

@shaunluttin
Copy link

@DustinCampbell Could you please invite me to the omnisharp-vscode channel on Slack. That way I can rant there instead. :trollface:

@DustinCampbell
Copy link
Member

Link is here: https://goo.gl/Ovnqr1

@DustinCampbell
Copy link
Member

A new (release)[https://github.com/OmniSharp/omnisharp-vscode/releases/tag/v1.5-beta6] is ready which includes a csharp.suppressDotnetRestoreNotification option.

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

No branches or pull requests

6 participants