search

nikeee / dot-vscode

MIT License
1 stars 1 forks source link

Two simple changes #6

Open CarlKnox opened 4 years ago

CarlKnox commented 4 years ago

Stephan's snippets don't work in my environment. Removing a few colons resolves this. vsce package fails when vscode is included in dependencies.

Resolves #3 Resolves #5

nikeee commented 4 years ago

Thanks for this PR!

I added the vscode types as dev dependencies. Turns out, it is not compatible with the current vscode.

This function does not return a value for every code path:

function getDisplayColumn(viewColumn: ViewColumn) {
    switch (viewColumn) {
        case ViewColumn.One:
            return ViewColumn.Two;
        case ViewColumn.Two:
        case ViewColumn.Three:
            return ViewColumn.Three;
    }
}

(ViewColumn now has more values than at the time it was written)

nikeee commented 4 years ago

Do you think the graph preview is something that should be in the domain of this extension?

I'm thinking about removing it because it would be much easier to maintain. Also, there are other package out there that provide a graph preview.

The graph preview is also the thing that prevents this PR from being merged (see above).