Skip to content

Commit de62536

Browse files
rchiodorchiodo
authored
Add support for palette/context menu commands to live share (#4574)
For #4520, #4197 Revamp how commands are passed around. Now they are mirror'd at the history level instead of the command level as something like 'Run Current Cell' is context sensitive (so you can't just send it to the host). Also fixed colors and the issue with the interactive window not coming up (because of file-matcher hanging). Did this here because it was making it a pain to debug this. Still more work to do: - Write functional test for liveshare - Try on actual remote boxes - Write systemtest for the colors <!-- If an item below does not apply to you, then go ahead and check it off as "done" and strikethrough the text, e.g.: - [x] ~Has unit tests & system/integration tests~ --> - [x] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR) - [x] Title summarizes what is changing - [x] Has a [news entry](https://github.com/Microsoft/vscode-python/tree/master/news) file (remember to thank yourself!) - [x] Has sufficient logging. - [ ] Has telemetry for enhancements. - [ ] Unit tests & system/integration tests are added/updated - [ ] [Test plan](https://github.com/Microsoft/vscode-python/blob/master/.github/test_plan.md) is updated as appropriate - [x] [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package-lock.json) has been regenerated by running `npm install` (if dependencies have changed)
1 parent bc1b054 commit de62536

46 files changed

Lines changed: 1804 additions & 1373 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎news/1 Enhancements/4520.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add support for palette commands to live share scenario.

‎package-lock.json‎

Lines changed: 139 additions & 37 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2172,7 +2172,6 @@
21722172
"arch": "^2.1.0",
21732173
"azure-storage": "^2.10.1",
21742174
"diff-match-patch": "^1.0.0",
2175-
"file-matcher": "^1.3.0",
21762175
"fs-extra": "^4.0.3",
21772176
"fuzzy": "^0.1.3",
21782177
"get-port": "^3.2.0",

‎src/client/datascience/codeCssGenerator.ts‎

Lines changed: 53 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,16 @@
22
// Licensed under the MIT License.
33
'use strict';
44
import { JSONArray, JSONObject, JSONValue } from '@phosphor/coreutils';
5-
import { FindOptions } from 'file-matcher';
65
import * as fs from 'fs-extra';
76
import { inject, injectable } from 'inversify';
87
import * as path from 'path';
98
import * as stripJsonComments from 'strip-json-comments';
109

1110
import { IWorkspaceService } from '../common/application/types';
12-
import { ICurrentProcess, ILogger } from '../common/types';
11+
import { ILogger } from '../common/types';
1312
import { EXTENSION_ROOT_DIR } from '../constants';
1413
import { Identifiers } from './constants';
15-
import { ICodeCssGenerator } from './types';
14+
import { ICodeCssGenerator, IThemeFinder } from './types';
1615

1716
// tslint:disable:no-any
1817

@@ -26,7 +25,7 @@ import { ICodeCssGenerator } from './types';
2625
export class CodeCssGenerator implements ICodeCssGenerator {
2726
constructor(
2827
@inject(IWorkspaceService) private workspaceService: IWorkspaceService,
29-
@inject(ICurrentProcess) private currentProcess: ICurrentProcess,
28+
@inject(IThemeFinder) private themeFinder: IThemeFinder,
3029
@inject(ILogger) private logger: ILogger) {
3130
}
3231

@@ -42,10 +41,12 @@ export class CodeCssGenerator implements ICodeCssGenerator {
4241

4342
// Then we have to find where the theme resources are loaded from
4443
if (theme) {
44+
this.logger.logInformation('Searching for token colors ...');
4545
const tokenColors = await this.findTokenColors(theme);
4646

4747
// The tokens object then contains the necessary data to generate our css
4848
if (tokenColors && font && fontSize) {
49+
this.logger.logInformation('Using colors to generate CSS ...');
4950
return this.generateCss(theme, tokenColors, font, fontSize, terminalCursor);
5051
}
5152
}
@@ -57,28 +58,23 @@ export class CodeCssGenerator implements ICodeCssGenerator {
5758
return '';
5859
}
5960

60-
private escapeThemeName(themeName: string) : string {
61-
return themeName.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&');
62-
}
63-
6461
private matchTokenColor(tokenColors: JSONArray, scope: string) : number {
6562
return tokenColors.findIndex((entry: any) => {
6663
if (entry) {
6764
const scopes = entry['scope'] as JSONValue;
68-
if (scopes && Array.isArray(scopes)) {
69-
if (scopes.find(v => v !== null && v !== undefined && v.toString() === scope)) {
65+
if (scopes) {
66+
const scopeArray = Array.isArray(scope) ? scopes as JSONArray : scopes.toString().split(',');
67+
if (scopeArray.find(v => v !== null && v !== undefined && v.toString().trim() === scope)) {
7068
return true;
7169
}
72-
} else if (scopes && scopes.toString() === scope) {
73-
return true;
7470
}
7571
}
7672

7773
return false;
7874
});
7975
}
8076

81-
private getScopeColor = (tokenColors: JSONArray, scope: string, secondary?: string): string => {
77+
private getScopeStyle = (tokenColors: JSONArray, scope: string, secondary?: string): { color: string; fontStyle: string } => {
8278
// Search through the scopes on the json object
8379
let match = this.matchTokenColor(tokenColors, scope);
8480
if (match < 0 && secondary) {
@@ -88,28 +84,31 @@ export class CodeCssGenerator implements ICodeCssGenerator {
8884
if (found !== null) {
8985
const settings = found['settings'];
9086
if (settings && settings !== null) {
91-
return settings['foreground'];
87+
const fontStyle = settings['fontStyle'] ? settings['fontStyle'] : 'normal';
88+
const foreground = settings['foreground'] ? settings['foreground'] : 'var(--vscode-editor-foreground)';
89+
90+
return { fontStyle, color: foreground };
9291
}
9392
}
9493

9594
// Default to editor foreground
96-
return 'var(--vscode-editor-foreground)';
95+
return { color: 'var(--vscode-editor-foreground)', fontStyle: 'normal' };
9796
}
9897

9998
// tslint:disable-next-line:max-func-body-length
10099
private generateCss(theme: string, tokenColors: JSONArray, fontFamily: string, fontSize: number, cursorType: string): string {
101100
const escapedThemeName = Identifiers.GeneratedThemeName;
102101

103102
// There's a set of values that need to be found
104-
const comment = this.getScopeColor(tokenColors, 'comment');
105-
const numeric = this.getScopeColor(tokenColors, 'constant.numeric');
106-
const stringColor = this.getScopeColor(tokenColors, 'string');
107-
const keyword = this.getScopeColor(tokenColors, 'keyword.control', 'keyword');
108-
const operator = this.getScopeColor(tokenColors, 'keyword.operator');
109-
const variable = this.getScopeColor(tokenColors, 'variable');
103+
const commentStyle = this.getScopeStyle(tokenColors, 'comment');
104+
const numericStyle = this.getScopeStyle(tokenColors, 'constant.numeric');
105+
const stringStyle = this.getScopeStyle(tokenColors, 'string');
106+
const keywordStyle = this.getScopeStyle(tokenColors, 'keyword.control', 'keyword');
107+
const operatorStyle = this.getScopeStyle(tokenColors, 'keyword.operator');
108+
const variableStyle = this.getScopeStyle(tokenColors, 'variable');
110109
// const atomic = this.getScopeColor(tokenColors, 'atomic');
111-
const builtin = this.getScopeColor(tokenColors, 'support.function');
112-
const punctuation = this.getScopeColor(tokenColors, 'punctuation');
110+
const builtinStyle = this.getScopeStyle(tokenColors, 'support.function');
111+
const punctuationStyle = this.getScopeStyle(tokenColors, 'punctuation');
113112

114113
const def = 'var(--vscode-editor-foreground)';
115114

@@ -122,7 +121,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
122121
// Use these values to fill in our format string
123122
return `
124123
:root {
125-
--code-comment-color: ${comment};
124+
--code-comment-color: ${commentStyle.color};
126125
--code-font-family: ${fontFamily};
127126
--code-font-size:${fontSize}px;
128127
}
@@ -131,19 +130,19 @@ export class CodeCssGenerator implements ICodeCssGenerator {
131130
.cm-link {text-decoration: underline;}
132131
.cm-strikethrough {text-decoration: line-through;}
133132
134-
.cm-s-${escapedThemeName} span.cm-keyword {color: ${keyword};}
135-
.cm-s-${escapedThemeName} span.cm-number {color: ${numeric};}
136-
.cm-s-${escapedThemeName} span.cm-def {color: ${def};}
137-
.cm-s-${escapedThemeName} span.cm-variable {color: ${variable};}
138-
.cm-s-${escapedThemeName} span.cm-punctuation {color: ${punctuation};}
133+
.cm-s-${escapedThemeName} span.cm-keyword {color: ${keywordStyle.color}; font-style: ${keywordStyle.fontStyle}; }
134+
.cm-s-${escapedThemeName} span.cm-number {color: ${numericStyle.color}; font-style: ${numericStyle.fontStyle}; }
135+
.cm-s-${escapedThemeName} span.cm-def {color: ${def}; }
136+
.cm-s-${escapedThemeName} span.cm-variable {color: ${variableStyle.color}; font-style: ${variableStyle.fontStyle}; }
137+
.cm-s-${escapedThemeName} span.cm-punctuation {color: ${punctuationStyle.color}; font-style: ${punctuationStyle.fontStyle}; }
139138
.cm-s-${escapedThemeName} span.cm-property,
140-
.cm-s-${escapedThemeName} span.cm-operator {color: ${operator};}
141-
.cm-s-${escapedThemeName} span.cm-variable-2 {color: ${variable};}
142-
.cm-s-${escapedThemeName} span.cm-variable-3, .cm-s-${theme} .cm-type {color: ${variable};}
143-
.cm-s-${escapedThemeName} span.cm-comment {color: ${comment};}
144-
.cm-s-${escapedThemeName} span.cm-string {color: ${stringColor};}
145-
.cm-s-${escapedThemeName} span.cm-string-2 {color: ${stringColor};}
146-
.cm-s-${escapedThemeName} span.cm-builtin {color: ${builtin};}
139+
.cm-s-${escapedThemeName} span.cm-operator {color: ${operatorStyle.color}; font-style: ${operatorStyle.fontStyle}; }
140+
.cm-s-${escapedThemeName} span.cm-variable-2 {color: ${variableStyle.color}; font-style: ${variableStyle.fontStyle}; }
141+
.cm-s-${escapedThemeName} span.cm-variable-3, .cm-s-${theme} .cm-type {color: ${variableStyle.color}; font-style: ${variableStyle.fontStyle}; }
142+
.cm-s-${escapedThemeName} span.cm-comment {color: ${commentStyle.color}; font-style: ${commentStyle.fontStyle}; }
143+
.cm-s-${escapedThemeName} span.cm-string {color: ${stringStyle.color}; font-style: ${stringStyle.fontStyle}; }
144+
.cm-s-${escapedThemeName} span.cm-string-2 {color: ${stringStyle.color}; font-style: ${stringStyle.fontStyle}; }
145+
.cm-s-${escapedThemeName} span.cm-builtin {color: ${builtinStyle.color}; font-style: ${builtinStyle.fontStyle}; }
147146
.cm-s-${escapedThemeName} div.CodeMirror-cursor ${cursorStyle}
148147
.cm-s-${escapedThemeName} div.CodeMirror-selected {background: var(--vscode-editor-selectionBackground) !important;}
149148
`;
@@ -171,42 +170,27 @@ export class CodeCssGenerator implements ICodeCssGenerator {
171170
return tokenColors;
172171
}
173172

173+
// Might also have a 'settings' object that equates to token colors
174+
const settings = theme['settings'] as JSONArray;
175+
if (settings && settings.length > 0) {
176+
return settings;
177+
}
178+
174179
return [];
175180
}
176181

177182
private findTokenColors = async (theme: string): Promise<JSONArray> => {
178-
const currentExe = this.currentProcess.execPath;
179-
let currentPath = path.dirname(currentExe);
180-
181-
// Should be somewhere under currentPath/resources/app/extensions inside of a json file
182-
let extensionsPath = path.join(currentPath, 'resources', 'app', 'extensions');
183-
if (!(await fs.pathExists(extensionsPath))) {
184-
// Might be on mac or linux. try a different path
185-
currentPath = path.resolve(currentPath, '../../../..');
186-
extensionsPath = path.join(currentPath, 'resources', 'app', 'extensions');
187-
}
188-
189-
// Search through all of the json files for the theme name
190-
const escapedThemeName = this.escapeThemeName(theme);
191-
const searchOptions: FindOptions = {
192-
path: extensionsPath,
193-
recursiveSearch: true,
194-
fileFilter: {
195-
fileNamePattern: '**/*.json',
196-
content: new RegExp(`[name|id][',"]:\\s*[',"]${escapedThemeName}[',"]`)
197-
}
198-
};
199-
// tslint:disable-next-line:no-require-imports
200-
const fm = require('file-matcher') as typeof import('file-matcher');
201-
const matcher = new fm.FileMatcher();
202183

203184
try {
204-
const results = await matcher.find(searchOptions);
185+
this.logger.logInformation('Attempting search for colors ...');
186+
const themeRoot = await this.themeFinder.findThemeRootJson(theme);
205187

206188
// Use the first result if we have one
207-
if (results && results.length > 0) {
189+
if (themeRoot) {
190+
this.logger.logInformation(`Loading colors from ${themeRoot} ...`);
191+
208192
// This should be the path to the file. Load it as a json object
209-
const contents = await fs.readFile(results[0], 'utf8');
193+
const contents = await fs.readFile(themeRoot, 'utf8');
210194
const json = JSON.parse(stripJsonComments(contents)) as JSONObject;
211195

212196
// There should be a theme colors section
@@ -217,7 +201,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
217201
if (!contributes) {
218202
const tokenColors = json['tokenColors'] as JSONObject;
219203
if (tokenColors) {
220-
return await this.readTokenColors(results[0]);
204+
return await this.readTokenColors(themeRoot);
221205
}
222206
}
223207

@@ -226,16 +210,19 @@ export class CodeCssGenerator implements ICodeCssGenerator {
226210

227211
// One of these (it's an array), should have our matching theme entry
228212
const index = themes.findIndex((e: any) => {
229-
return e !== null && e['id'] === theme;
213+
return e !== null && (e['id'] === theme || e['name'] === theme);
230214
});
231215

232216
const found = index >= 0 ? themes[index] as any : null;
233217
if (found !== null) {
234218
// Then the path entry should contain a relative path to the json file with
235219
// the tokens in it
236-
const themeFile = path.join(path.dirname(results[0]), found['path']);
220+
const themeFile = path.join(path.dirname(themeRoot), found['path']);
221+
this.logger.logInformation(`Reading colors from ${themeFile}`);
237222
return await this.readTokenColors(themeFile);
238223
}
224+
} else {
225+
this.logger.logWarning(`Color theme ${theme} not found. Using default colors.`);
239226
}
240227
} catch (err) {
241228
// Swallow any exceptions with searching or parsing

‎src/client/datascience/commandBroker.ts‎

Lines changed: 0 additions & 81 deletions
This file was deleted.

0 commit comments

Comments
 (0)