Skip to content

Commit ea815f7

Browse files
committed
adds comments, removes a logic block that wasn't doing anything
I'm fairly certain that we can remove ``` if (!prevProps?.unsavedChanges) { setTimeout(() => setUnsavedChanges(false), 400); } ``` I looked at the git blame and it looks like the intention was to stop setting unsavedchanges to false when the files switched, but i think the solve that was implemented 9 years ago did something like "if c == false, c = false" and we should be able to safely remove it instead. reference commit: 77e2f5b
1 parent 6d7ef6d commit ea815f7

File tree

2 files changed

+18
-11
lines changed

2 files changed

+18
-11
lines changed

client/modules/IDE/components/Editor/codemirror.js

+16-9
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const INDENTATION_AMOUNT = 2;
3737

3838
emmet(CodeMirror);
3939

40+
/** This is a custom React hook that manages CodeMirror state. */
4041
export default function useCodeMirror({
4142
theme,
4243
lineNumbers,
@@ -56,7 +57,9 @@ export default function useCodeMirror({
5657
fontSize,
5758
onUpdateLinting
5859
}) {
60+
// The codemirror instance.
5961
const cmInstance = useRef();
62+
// The current codemirror files.
6063
const docs = useRef();
6164

6265
function onKeyUp() {
@@ -89,6 +92,7 @@ export default function useCodeMirror({
8992
const fileId = useRef();
9093
fileId.current = file.id;
9194

95+
// When the file changes, update the file content and save status.
9296
function onChange() {
9397
setUnsavedChanges(true);
9498
hideRuntimeErrorWarning();
@@ -100,6 +104,8 @@ export default function useCodeMirror({
100104
}
101105
const debouncedOnChange = debounce(onChange, 1000);
102106

107+
// When the container component enters the DOM, we want this function
108+
// to be called so we can setup the CodeMirror instance with the container.
103109
function setupCodeMirrorOnContainerMounted(container) {
104110
cmInstance.current = CodeMirror(container, {
105111
theme: `p5-${theme}`,
@@ -169,13 +175,15 @@ export default function useCodeMirror({
169175
[`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+.
170176
});
171177

178+
// Setup the event listeners on the CodeMirror instance.
172179
cmInstance.current.on('change', debouncedOnChange);
173180
cmInstance.current.on('keyup', onKeyUp);
174181
cmInstance.current.on('keydown', onKeyDown);
175182

176183
cmInstance.current.getWrapperElement().style['font-size'] = `${fontSize}px`;
177184
}
178185

186+
// When settings change, we pass those changes into CodeMirror.
179187
useEffect(() => {
180188
cmInstance.current.getWrapperElement().style['font-size'] = `${fontSize}px`;
181189
}, [fontSize]);
@@ -192,7 +200,8 @@ export default function useCodeMirror({
192200
cmInstance.current.setOption('autoCloseBrackets', autocloseBracketsQuotes);
193201
}, [autocloseBracketsQuotes]);
194202

195-
const initializeDocuments = () => {
203+
// Initializes the files as CodeMirror documents.
204+
function initializeDocuments() {
196205
docs.current = {};
197206
files.forEach((currentFile) => {
198207
if (currentFile.name !== 'root') {
@@ -202,12 +211,13 @@ export default function useCodeMirror({
202211
);
203212
}
204213
});
205-
};
214+
}
206215

207-
useEffect(() => {
208-
initializeDocuments();
209-
}, [files]);
216+
// When the files change, reinitialize the documents.
217+
useEffect(initializeDocuments, [files]);
210218

219+
// When the file changes, we change the file mode and
220+
// make the CodeMirror call to swap out the document.
211221
useEffectWithComparison(
212222
(_, prevProps) => {
213223
const fileMode = getFileMode(file.name);
@@ -226,10 +236,6 @@ export default function useCodeMirror({
226236
}
227237
cmInstance.current.focus();
228238

229-
if (!prevProps?.unsavedChanges) {
230-
setTimeout(() => setUnsavedChanges(false), 400);
231-
}
232-
233239
for (let i = 0; i < cmInstance.current.lineCount(); i += 1) {
234240
cmInstance.current.removeLineClass(
235241
i,
@@ -241,6 +247,7 @@ export default function useCodeMirror({
241247
[file.id]
242248
);
243249

250+
// Remove the CM listeners on component teardown.
244251
function teardownCodeMirror() {
245252
cmInstance.current.off('keyup', onKeyUp);
246253
cmInstance.current.off('change', debouncedOnChange);

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,11 @@ function Editor({
146146
}, []);
147147

148148
useEffect(() => {
149-
// close the hinter window once the preference is turned off
149+
// Close the hinter window once the preference is turned off
150150
if (!autocompleteHinter) hideHinter(cmInstance.current);
151151
}, [autocompleteHinter]);
152152

153-
// TODO: test this
153+
// Updates the error console.
154154
useEffectWithComparison(
155155
(_, prevProps) => {
156156
if (runtimeErrorWarningVisible) {

0 commit comments

Comments
 (0)