Skip to content

Commit a43aac6

Browse files
diningPhilosopher64prabhakk-mw
authored andcommitted
Addressed review comments
1 parent 5184d48 commit a43aac6

31 files changed

+898
-1213
lines changed

src/jupyter_matlab_kernel/kernels/__init__.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,4 @@
22

33
from .jsp_kernel import MATLABKernelUsingJSP
44
from .mpm_kernel import MATLABKernelUsingMPM
5-
from .kernel_factory import KernelFactory
6-
7-
__all__ = ["MATLABKernelUsingJSP", "MATLABKernelUsingMPM", "KernelFactory"]
5+
from .kernel_factory import KernelFactory

src/jupyter_matlab_kernel/kernels/base_kernel.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import time
1414
from logging import Logger
1515
from pathlib import Path
16-
from typing import Optional
1716

1817
import aiohttp
1918
import aiohttp.client_exceptions
@@ -78,7 +77,7 @@ def _fetch_jupyter_base_url(parent_pid: str, logger: Logger) -> str:
7877
return ""
7978

8079

81-
def _get_parent_pid(logger) -> int:
80+
def _get_parent_pid() -> int:
8281
"""
8382
Retrieves the parent process ID (PID) of the Kernel process.
8483

src/jupyter_matlab_kernel/kernels/kernel_factory.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
import os
44
from typing import Union
55

6-
from jupyter_matlab_kernel.kernels.jsp_kernel import MATLABKernelUsingJSP
7-
from jupyter_matlab_kernel.kernels.mpm_kernel import MATLABKernelUsingMPM
6+
from jupyter_matlab_kernel.kernels import MATLABKernelUsingJSP
7+
from jupyter_matlab_kernel.kernels import MATLABKernelUsingMPM
88

99

1010
class KernelFactory:

src/jupyter_matlab_kernel/kernels/labextension_comm/communication.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ def __init__(self, kernel):
1111

1212
def comm_open(self, stream, ident, msg):
1313
"""Handler to execute when labextension sends a message with 'comm_open' type ."""
14+
15+
# As per jupyter messaging protocol https://jupyter-client.readthedocs.io/en/latest/messaging.html#custom-messages
16+
# 'content' will be present in msg, 'comm_id' and 'target_name' will be present in content.
17+
1418
content = msg["content"]
1519
comm_id = content["comm_id"]
1620
target_name = content["target_name"]
@@ -27,6 +31,9 @@ async def comm_msg(self, stream, ident, msg):
2731

2832
def comm_close(self, stream, ident, msg):
2933
"""Handler to execute when labextension sends a message with 'comm_close' type."""
34+
35+
# As per jupyter messaging protocol https://jupyter-client.readthedocs.io/en/latest/messaging.html#custom-messages
36+
# 'content' will be present in msg, 'comm_id' and 'data' will be present in content.
3037
content = msg["content"]
3138
comm_id = content["comm_id"]
3239
comm = self.comms.get(comm_id)
@@ -36,4 +43,4 @@ def comm_close(self, stream, ident, msg):
3643
del self.comms[comm_id]
3744

3845
else:
39-
self.log.warning(f"Attempted to close unknown comm_id: {comm_id}")
46+
self.log.debug(f"Attempted to close unknown comm_id: {comm_id}")

src/jupyter_matlab_kernel/mwi_comm_helpers.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,9 @@ async def _send_jupyter_request_to_matlab(self, request_type, inputs, http_clien
385385
resp = await self._send_feval_request_to_matlab(
386386
http_client, "processJupyterKernelRequest", 1, *inputs
387387
)
388+
389+
# The 'else' condition is an artifact and is present here incase we ever want to test
390+
# eval execution.
388391
else:
389392
user_mcode = inputs[2]
390393
# Construct a string which can be evaluated in MATLAB. For example
Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,41 @@
1-
21
{
32
"root": true,
43
"parser": "@typescript-eslint/parser",
54
"parserOptions": {
6-
"ecmaVersion": 6,
7-
"sourceType": "module"
5+
"ecmaVersion": 6,
6+
"sourceType": "module"
87
},
98
"env": {
109
"jest": true
1110
},
1211
"plugins": ["@typescript-eslint"],
1312
"extends": ["standard"],
1413
"rules": {
15-
"indent": ["error", 4, { "SwitchCase": 1 }],
16-
"semi": ["error", "always"],
17-
"no-extra-semi": "error",
18-
"quote-props": "warn",
19-
"dot-notation": "warn",
20-
"object-curly-newline": "warn",
21-
"multiline-ternary": "warn",
22-
"prefer-const": "warn",
23-
"no-prototype-builtins": "warn",
24-
"array-callback-return": "warn",
25-
"array-bracket-spacing": "warn",
26-
"quotes": "warn",
27-
"lines-between-class-members": "warn",
28-
"no-empty": "warn",
29-
"prefer-regex-literals": "warn",
30-
"no-useless-catch": "warn",
31-
"no-case-declarations": "warn",
32-
"computed-property-spacing": "warn",
33-
"no-async-promise-executor": "warn",
34-
"no-unused-vars": "warn",
35-
"no-unreachable-loop": "warn",
36-
"no-void": "warn",
37-
"import/no-webpack-loader-syntax": "warn",
38-
"node/no-callback-literal": ["off", "warn"],
39-
"node/handle-callback-err": ["off", "warn"],
40-
"node/no-deprecated-api": ["off", "warn"]
14+
"indent": ["error", 4, { "SwitchCase": 1 }],
15+
"semi": ["error", "always"],
16+
"no-extra-semi": "error",
17+
"quote-props": "warn",
18+
"dot-notation": "warn",
19+
"object-curly-newline": "warn",
20+
"multiline-ternary": "warn",
21+
"prefer-const": "warn",
22+
"no-prototype-builtins": "warn",
23+
"array-callback-return": "warn",
24+
"array-bracket-spacing": "warn",
25+
"quotes": "warn",
26+
"lines-between-class-members": "warn",
27+
"no-empty": "warn",
28+
"prefer-regex-literals": "warn",
29+
"no-useless-catch": "warn",
30+
"no-case-declarations": "warn",
31+
"computed-property-spacing": "warn",
32+
"no-async-promise-executor": "warn",
33+
"no-unused-vars": "warn",
34+
"no-unreachable-loop": "warn",
35+
"no-void": "warn",
36+
"import/no-webpack-loader-syntax": "warn",
37+
"node/no-callback-literal": ["off", "warn"],
38+
"node/handle-callback-err": ["off", "warn"],
39+
"node/no-deprecated-api": ["off", "warn"]
4140
}
4241
}
43-
44-

src/jupyter_matlab_labextension/.styleintrc.json

Whitespace-only changes.
Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
// Copyright 2025 The MathWorks, Inc.
22
module.exports = {
3-
preset: 'ts-jest',
4-
testEnvironment: 'node',
5-
testMatch: ['**/tests/**/*.ts?(x)', '**/?(*.)+(spec|test).ts?(x)'],
6-
testPathIgnorePatterns: ['/node_modules/', '/src/tests/jest-setup.ts'],
7-
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
8-
setupFilesAfterEnv: ['<rootDir>/src/tests/jest-setup.ts'],
3+
preset: "ts-jest",
4+
testEnvironment: "node",
5+
testMatch: ["**/tests/**/*.ts?(x)", "**/?(*.)+(spec|test).ts?(x)"],
6+
testPathIgnorePatterns: ["/node_modules/", "/src/tests/jest-setup.ts"],
7+
moduleFileExtensions: ["ts", "tsx", "js", "jsx", "json", "node"],
8+
setupFilesAfterEnv: ["<rootDir>/src/tests/jest-setup.ts"],
99
transform: {
10-
'^.+\\.(ts|tsx)$': 'ts-jest'
10+
"^.+\\.(ts|tsx)$": "ts-jest",
1111
},
1212
transformIgnorePatterns: [
13-
'/node_modules/(?!(@jupyterlab)/)' // Transform @jupyterlab packages
13+
"/node_modules/(?!(@jupyterlab)/)", // Transform @jupyterlab packages
1414
],
1515
moduleNameMapper: {
1616
// Mock @jupyterlab/ui-components to avoid ES modules issues
17-
'@jupyterlab/ui-components': '<rootDir>/src/tests/mocks/ui-components.js'
18-
}
19-
};
17+
"@jupyterlab/ui-components": "<rootDir>/src/tests/mocks/ui-components.js",
18+
},
19+
};

src/jupyter_matlab_labextension/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@
8585
"prettier": "^2.8.7",
8686
"rimraf": "^4.4.1",
8787
"semver": ">=5.7.2",
88-
"stylelint": "^16.18.0",
8988
"ts-jest": "^29.2.5",
9089
"typescript": "~5.0.2",
9190
"ws": "^7.5.10",

src/jupyter_matlab_labextension/src/codemirror-lang-matlab/codemirror-lang-matlab.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,21 @@
11
// Copyright 2024-2025 The MathWorks, Inc.
22

33
import { parser } from '../lezer-matlab/dist/index';
4-
import { indentNodeProp, LanguageSupport, LRLanguage, TreeIndentContext } from '@codemirror/language';
4+
import {
5+
indentNodeProp,
6+
LanguageSupport,
7+
LRLanguage,
8+
TreeIndentContext
9+
} from '@codemirror/language';
510
import { lineIndent, getDedentPattern } from './indent-matlab';
611

712
function determineLineIndent (context: TreeIndentContext) {
8-
if (context.pos === 0) { return null; }
13+
if (context.pos === 0) {
14+
return null;
15+
}
916
const currentLine = context.lineAt(context.pos);
10-
const previousLine = currentLine.text.length === 0
17+
const previousLine =
18+
currentLine.text.length === 0
1119
? context.lineAt(context.pos, -1) // Look to the left of the simulated line break.
1220
: context.lineAt(context.pos - 1); // Not on a simulated line break, so step back to the previous line.
1321
if (previousLine === null || currentLine === null) {

0 commit comments

Comments
 (0)