Skip to content

Commit 0d5121d

Browse files
kammeowskammeows
committed
rename key trigger for mac users, add renaming in keyboard shortcuts, remove eslint-disabled from unneccessary files among other minor changes
1 parent 87dc886 commit 0d5121d

File tree

10 files changed

+189
-159
lines changed

10 files changed

+189
-159
lines changed

‎client/index.jsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ const App = () => (
4343
</div>
4444
);
4545

46+
// This prevents crashes in test environments (like Jest) where document.getElementById('root') may return null.
4647
const rootEl = document.getElementById('root');
4748
if (rootEl) {
4849
render(

‎client/modules/IDE/components/Editor/index.jsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ import IconButton from '../../../../common/IconButton';
7575

7676
import contextAwareHinter from '../../../../utils/contextAwareHinter';
7777
import showRenameDialog from '../../../../utils/showRenameDialog';
78-
import { handleRename } from '../../../../utils/rename-variable';
78+
import handleRename from '../../../../utils/rename-variable';
7979
import { jumpToDefinition } from '../../../../utils/jump-to-definition';
8080
import { ensureAriaLiveRegion } from '../../../../utils/ScreenReaderHelper';
8181
import isMac from '../../../../utils/device';
@@ -171,6 +171,8 @@ class Editor extends React.Component {
171171
}
172172
});
173173

174+
const renameKey = isMac() ? 'Ctrl-F2' : 'F2';
175+
174176
const replaceCommand =
175177
metaKey === 'Ctrl' ? `${metaKey}-H` : `${metaKey}-Option-F`;
176178
this._cm.setOption('extraKeys', {
@@ -189,7 +191,7 @@ class Editor extends React.Component {
189191
[`Shift-${metaKey}-E`]: (cm) => {
190192
cm.getInputField().blur();
191193
},
192-
F2: (cm) => this.renameVariable(cm),
194+
[renameKey]: (cm) => this.renameVariable(cm),
193195
[`Shift-Tab`]: false,
194196
[`${metaKey}-Enter`]: () => null,
195197
[`Shift-${metaKey}-Enter`]: () => null,
@@ -227,6 +229,7 @@ class Editor extends React.Component {
227229
}
228230

229231
this._cm.on('keydown', (_cm, e) => {
232+
// Skip hinting if the user is pasting (Ctrl/Cmd+V) or using modifier keys (Ctrl/Alt)
230233
if (
231234
((e.ctrlKey || e.metaKey) && e.key === 'v') ||
232235
e.ctrlKey ||

‎client/modules/IDE/components/KeyboardShortcutModal.jsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ function KeyboardShortcutModal() {
88
metaKey === 'Ctrl' ? `${metaKeyName} + H` : `${metaKeyName} + ⌥ + F`;
99
const newFileCommand =
1010
metaKey === 'Ctrl' ? `${metaKeyName} + Alt + N` : `${metaKeyName} + ⌥ + N`;
11+
const renameCommand = metaKey === 'Ctrl' ? 'F2' : 'Ctrl + F2';
1112
return (
1213
<div className="keyboard-shortcuts">
1314
<h3 className="keyboard-shortcuts__title">
@@ -75,6 +76,10 @@ function KeyboardShortcutModal() {
7576
<span className="keyboard-shortcut__command">{newFileCommand}</span>
7677
<span>{t('KeyboardShortcuts.CodeEditing.CreateNewFile')}</span>
7778
</li>
79+
<li className="keyboard-shortcut-item">
80+
<span className="keyboard-shortcut__command">{renameCommand}</span>
81+
<span>{t('KeyboardShortcuts.CodeEditing.RenameVariable')}</span>
82+
</li>
7883
</ul>
7984
<h3 className="keyboard-shortcuts__title">
8085
{t('KeyboardShortcuts.General')}

‎client/utils/ScreenReaderHelper.jsx‎ renamed to ‎client/utils/ScreenReaderHelper.js‎

File renamed without changes.

‎client/utils/contextAwareHinter.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/* eslint-disable */
21
import getContext from './getContext';
32
import p5CodeAstAnalyzer from './p5CodeAstAnalyzer';
43
import classMap from './p5-instance-methods-and-creators.json';
@@ -119,7 +118,7 @@ export default function contextAwareHinter(cm, options = {}) {
119118
const isFunc =
120119
scopeToDeclaredVarsMap[currentContext]?.[varName] === 'fun' ||
121120
(!scopeToDeclaredVarsMap[currentContext]?.[varName] &&
122-
scopeToDeclaredVarsMap['global']?.[varName] === 'fun');
121+
scopeToDeclaredVarsMap.global?.[varName] === 'fun');
123122

124123
const baseItem = isFunc
125124
? { ...userDefinedFunctionMetadata[varName] }

‎client/utils/jump-to-def-helper.js‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
/* eslint-disable */
2-
import p5CodeAstAnalyzer from './p5CodeAstAnalyzer';
31
import * as parser from '@babel/parser';
42
import announceToScreenReader from './ScreenReaderHelper';
3+
54
const traverse = require('@babel/traverse').default;
65

76
export function getScriptLoadOrder(files) {
@@ -10,19 +9,21 @@ export function getScriptLoadOrder(files) {
109

1110
const scriptRegex = /<script\s+[^>]*src=["']([^"']+)["']/g;
1211
const scripts = [];
13-
let match;
14-
while ((match = scriptRegex.exec(indexHtmlFile.content)) !== null) {
12+
let match = scriptRegex.exec(indexHtmlFile.content);
13+
while (match !== null) {
1514
scripts.push(match[1]);
15+
match = scriptRegex.exec(indexHtmlFile.content);
1616
}
17+
1718
return scripts;
1819
}
1920

2021
export function buildProjectSymbolTable(files, scriptOrder) {
2122
const symbolTable = {};
2223

23-
for (const scriptName of scriptOrder) {
24+
scriptOrder.forEach((scriptName) => {
2425
const file = files.find((f) => f.name.endsWith(scriptName));
25-
if (!file) continue;
26+
if (!file) return;
2627

2728
let ast;
2829
try {
@@ -31,7 +32,7 @@ export function buildProjectSymbolTable(files, scriptOrder) {
3132
plugins: ['jsx', 'typescript']
3233
});
3334
} catch (e) {
34-
continue;
35+
return;
3536
}
3637

3738
traverse(ast, {
@@ -54,7 +55,7 @@ export function buildProjectSymbolTable(files, scriptOrder) {
5455
}
5556
}
5657
});
57-
}
58+
});
5859

5960
return symbolTable;
6061
}

‎client/utils/rename-variable.js‎

Lines changed: 56 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
/* eslint-disable */
2-
import { includes } from 'lodash';
31
import p5CodeAstAnalyzer from './p5CodeAstAnalyzer';
42
import {
53
getContext,
@@ -8,20 +6,10 @@ import {
86
getClassContext,
97
isThisReference
108
} from './renameVariableHelper';
11-
const parser = require('@babel/parser');
12-
const traverse = require('@babel/traverse').default;
139

14-
export function handleRename(fromPos, oldName, newName, cm) {
15-
if (!cm) {
16-
return;
17-
}
18-
const ast = getAST(cm);
19-
startRenaming(cm, ast, fromPos, newName, oldName);
20-
}
10+
const traverse = require('@babel/traverse').default;
2111

2212
function startRenaming(cm, ast, fromPos, newName, oldName) {
23-
const code = cm.getValue();
24-
const fromIndex = cm.indexFromPos(fromPos);
2513
const {
2614
scopeToDeclaredVarsMap = {},
2715
userDefinedClassMetadata = {},
@@ -69,8 +57,7 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
6957
parent.type === 'ArrowFunctionExpression') &&
7058
parent.params.some((p) => p.type === 'Identifier' && p.name === oldName)
7159
) {
72-
if (parent.params.includes(node)) {
73-
} else {
60+
if (!parent.params.includes(node)) {
7461
return;
7562
}
7663
}
@@ -125,29 +112,30 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
125112
isThis === isBaseThis &&
126113
baseContext === thisContext
127114
) {
128-
for (const [methodName, vars] of Object.entries(
129-
classMeta.methodVars || {}
130-
)) {
131-
if (!vars.includes(oldName) && thisContext === methodName) {
132-
shouldRenameMethodVar = true;
115+
Object.entries(classMeta.methodVars || {}).forEach(
116+
([methodName, vars]) => {
117+
if (!vars.includes(oldName) && thisContext === methodName) {
118+
const shouldRenameMethodVar = true;
119+
}
133120
}
134-
}
121+
);
122+
135123
shouldRename =
136124
thisContext === baseContext &&
137-
(currentMethodName == 'constructor' || shouldRenameGlobalVar);
125+
(currentMethodName === 'constructor' || shouldRenameGlobalVar);
138126
} else {
139-
for (const [methodName, vars] of Object.entries(
140-
classMeta.methodVars || {}
141-
)) {
142-
if (
143-
!vars.includes(oldName) &&
144-
baseMethodName === currentMethodName &&
145-
isThis === isBaseThis &&
146-
baseContext === thisContext
147-
) {
148-
shouldRename = true;
127+
Object.entries(classMeta.methodVars || {}).forEach(
128+
([methodName, vars]) => {
129+
if (
130+
!vars.includes(oldName) &&
131+
baseMethodName === currentMethodName &&
132+
isThis === isBaseThis &&
133+
baseContext === thisContext
134+
) {
135+
shouldRename = true;
136+
}
149137
}
150-
}
138+
);
151139
}
152140

153141
// Rename constructor parameters, class fields, and method variables carefully
@@ -161,21 +149,31 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
161149

162150
if (!(thisContext in userDefinedClassMetadata)) {
163151
const thisScopeVars = scopeToDeclaredVarsMap[thisContext] || {};
164-
shouldRename = isGlobal && !thisScopeVars.hasOwnProperty(oldName);
152+
shouldRename =
153+
isGlobal &&
154+
!Object.prototype.hasOwnProperty.call(thisScopeVars, oldName);
165155
}
166156
shouldRenameGlobalVar = isGlobal && thisContext === 'global';
167157
}
168158
// Handle renaming outside classes
169159
else {
170160
const thisScopeVars = scopeToDeclaredVarsMap[thisContext] || {};
171161
const baseScopeVars = scopeToDeclaredVarsMap[baseContext] || {};
172-
const globalScopeVars = scopeToDeclaredVarsMap['global'] || {};
162+
const globalScopeVars = scopeToDeclaredVarsMap.global || {};
173163

174164
const isInBaseScope = thisContext === baseContext;
175165
const isShadowedLocally =
176-
!isInBaseScope && thisScopeVars.hasOwnProperty(oldName);
177-
const isDeclaredInBaseScope = baseScopeVars.hasOwnProperty(oldName);
178-
const isDeclaredGlobally = globalScopeVars.hasOwnProperty(oldName);
166+
!isInBaseScope &&
167+
Object.prototype.hasOwnProperty.call(thisScopeVars, oldName);
168+
const isDeclaredInBaseScope = Object.prototype.hasOwnProperty.call(
169+
baseScopeVars,
170+
oldName
171+
);
172+
const isDeclaredGlobally = Object.prototype.hasOwnProperty.call(
173+
globalScopeVars,
174+
oldName
175+
);
176+
179177
const isThisGlobal = isGlobalReference(
180178
oldName,
181179
thisContext,
@@ -223,23 +221,24 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
223221
!(currentMethodName === 'constructor')
224222
) {
225223
const classMeta = userDefinedClassMetadata[thisContext];
226-
for (const [methodName, vars] of Object.entries(
227-
classMeta.methodVars || {}
228-
)) {
229-
if (!vars.includes(oldName) && methodName === currentMethodName) {
230-
shouldRename = true;
224+
Object.entries(classMeta.methodVars || {}).forEach(
225+
([methodName, vars]) => {
226+
if (!vars.includes(oldName) && methodName === currentMethodName) {
227+
shouldRename = true;
228+
}
231229
}
232-
}
230+
);
233231
} else {
234232
shouldRename =
235233
isInBaseScope ||
236234
(!isShadowedLocally &&
237-
thisScopeVars.hasOwnProperty(oldName) === {} &&
235+
Object.prototype.hasOwnProperty.call(thisScopeVars, oldName) ===
236+
{} &&
238237
baseContext === 'global') ||
239238
(baseContext === 'global' &&
240-
!thisScopeVars.hasOwnProperty(oldName)) ||
239+
!Object.prototype.hasOwnProperty.call(thisScopeVars, oldName)) ||
241240
(isDeclaredGlobally &&
242-
!thisScopeVars.hasOwnProperty(oldName) &&
241+
!Object.prototype.hasOwnProperty.call(thisScopeVars, oldName) &&
243242
!isDeclaredInBaseScope);
244243

245244
shouldRenameGlobalVar =
@@ -261,8 +260,16 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
261260
);
262261

263262
cm.operation(() => {
264-
for (const { from, to } of replacements) {
263+
replacements.forEach(({ from, to }) => {
265264
cm.replaceRange(newName, from, to);
266-
}
265+
});
267266
});
268267
}
268+
269+
export default function handleRename(fromPos, oldName, newName, cm) {
270+
if (!cm) {
271+
return;
272+
}
273+
const ast = getAST(cm);
274+
startRenaming(cm, ast, fromPos, newName, oldName);
275+
}

‎client/utils/renameVariableHelper.jsx‎ renamed to ‎client/utils/renameVariableHelper.js‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
/* eslint-disable */
2-
import { includes } from 'lodash';
3-
import p5CodeAstAnalyzer from './p5CodeAstAnalyzer';
41
const parser = require('@babel/parser');
52
const traverse = require('@babel/traverse').default;
63

@@ -12,13 +9,16 @@ export function isGlobalReference(
129
baseClassContext,
1310
isBaseThis
1411
) {
15-
const globalVars = scopeToDeclaredVarsMap['global'] || {};
16-
if (!globalVars.hasOwnProperty(varName)) {
12+
const globalVars = scopeToDeclaredVarsMap.global || {};
13+
if (!Object.prototype.hasOwnProperty.call(globalVars, varName)) {
1714
return false;
1815
}
1916

2017
const localVars = scopeToDeclaredVarsMap[context] || {};
21-
if (!(context === 'global') && localVars.hasOwnProperty(varName)) {
18+
if (
19+
!(context === 'global') &&
20+
Object.prototype.hasOwnProperty.call(localVars, varName)
21+
) {
2222
return false;
2323
}
2424

@@ -40,7 +40,10 @@ export function isGlobalReference(
4040
if (
4141
methodName &&
4242
classMeta.methodVars?.[methodName] &&
43-
classMeta.methodVars[methodName].hasOwnProperty(varName)
43+
Object.prototype.hasOwnProperty.call(
44+
classMeta.methodVars[methodName],
45+
varName
46+
)
4447
) {
4548
return false;
4649
}
@@ -143,18 +146,14 @@ export function getContext(
143146

144147
export function getAST(cm) {
145148
const code = cm.getValue();
146-
const cursor = cm.getCursor();
147-
const offset = cm.indexFromPos(cursor);
148149

149-
let ast;
150150
try {
151-
ast = parser.parse(code, {
151+
return parser.parse(code, {
152152
sourceType: 'script',
153153
plugins: ['jsx', 'typescript']
154154
});
155-
return ast;
156155
} catch (e) {
157-
return;
156+
return null;
158157
}
159158
}
160159

@@ -176,14 +175,15 @@ export function getClassContext(cm, ast, fromPos) {
176175
const className = node.id?.name || '(anonymous class)';
177176
classContext = className;
178177

179-
const methodPath = path.get('body.body').find((p) => {
180-
return (
181-
(p.node.type === 'ClassMethod' ||
182-
p.node.type === 'ClassPrivateMethod') &&
183-
offset >= p.node.start &&
184-
offset <= p.node.end
178+
const methodPath = path
179+
.get('body.body')
180+
.find(
181+
(p) =>
182+
(p.node.type === 'ClassMethod' ||
183+
p.node.type === 'ClassPrivateMethod') &&
184+
offset >= p.node.start &&
185+
offset <= p.node.end
185186
);
186-
});
187187

188188
if (methodPath) {
189189
let methodName;

0 commit comments

Comments
 (0)