tbonelee commented on code in PR #5044:
URL: https://github.com/apache/zeppelin/pull/5044#discussion_r2304841421
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts:
##########
@@ -61,6 +61,7 @@ export class NotebookParagraphCodeEditorComponent implements
OnChanges, OnDestro
private monacoDisposables: IDisposable[] = [];
height = 18;
interpreterName?: string;
+ editorSettingTriggerAllowed: boolean = false;
Review Comment:
Could we perhaps add a comment explaining why this property is introduced
here?
##########
zeppelin-web-angular/src/app/core/paragraph-base/paragraph-base.ts:
##########
@@ -142,6 +144,8 @@ export abstract class ParagraphBase extends
MessageListenersManager {
}
this.cdr.markForCheck();
});
+ this.notebookParagraphCodeEditorComponent.editorSettingTriggerAllowed =
true;
Review Comment:
We should handle the case where `this.notebookParagraphCodeEditorComponent`
is undefined
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts:
##########
@@ -109,6 +109,10 @@ export class NotebookComponent extends
MessageListenersManager implements OnInit
@MessageListener(OP.INTERPRETER_BINDINGS)
loadInterpreterBindings(data:
MessageReceiveDataTypeMap[OP.INTERPRETER_BINDINGS]) {
+ this.listOfNotebookParagraphComponent.forEach(item => {
+ item.notebookParagraphCodeEditorComponent.editorSettingTriggerAllowed =
true;
Review Comment:
We should handle some case where `notebookParagraphCodeEditorComponent` is
undefined.
Also, should we trigger editor settings for all paragraphs here? Would
calling `this.setParagraphMode(true)` in `editor.onDidChangeModelContent()` be
sufficient to get editor settings only to the paragraph whose interpreter has
changed?
##########
zeppelin-web-angular/src/app/core/paragraph-base/paragraph-base.ts:
##########
@@ -54,6 +55,7 @@ export abstract class ParagraphBase extends
MessageListenersManager {
// Initialized by `ViewChildren` in the class which extends ParagraphBase
notebookParagraphResultComponents!:
QueryList<NotebookParagraphResultComponent>;
+ notebookParagraphCodeEditorComponent!: NotebookParagraphCodeEditorComponent;
Review Comment:
I think this non-null assertion should be removed and changed to optional. I
found it to be `undefined` when `paragraph.config.editorHide === true` in the
`%md` editor case. Maybe you could check some errors after running `%md`
interpreter
I mistakenly added non-null assertion for
`notebookParagraphCodeEditorComponent` in `NotebookParagraphComponent` in a
previous commit, but that was incorrect. Sorry for the confusion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
