Copilot commented on code in PR #840:
URL:
https://github.com/apache/skywalking-banyandb/pull/840#discussion_r2501807684
##########
ui/src/components/BydbQL/Index.vue:
##########
@@ -273,19 +274,79 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`);
error.value = null;
executionTime.value = 0;
}
- // Setup keyboard shortcuts for CodeMirror
- onMounted(() => {
- nextTick(() => {
- // Find the CodeMirror instance and add keyboard shortcuts
- const codeMirrorElement = document.querySelector('.query-input
.CodeMirror');
- if (codeMirrorElement && codeMirrorElement.CodeMirror) {
- const cm = codeMirrorElement.CodeMirror;
- cm.setOption('extraKeys', {
- 'Ctrl-Enter': executeQuery,
- 'Cmd-Enter': executeQuery,
- });
- }
+
+ function onCodeMirrorReady(cm) {
+ codeMirrorInstance.value = cm;
+ cm.setOption('extraKeys', {
+ 'Ctrl-Enter': executeQuery,
+ 'Cmd-Enter': executeQuery,
+ 'Ctrl-Space': 'autocomplete',
});
+ }
+
+ // Fetch groups and schemas for autocomplete
+ async function fetchSchemaData() {
+ try {
+ const groupResponse = await getGroupList();
+ if (groupResponse.error) {
+ console.error('Failed to fetch groups:', groupResponse.error);
+ return;
+ }
+
+ // Only include groups with valid catalog types (property, stream,
trace, measure, topn)
+ const groups = (groupResponse.group || [])
+ .filter((g) => CatalogToGroupType[g.catalog])
+ .map((g) => g.metadata.name);
+ const schemas = {
+ stream: [],
+ measure: [],
+ trace: [],
+ property: [],
+ topn: [],
+ };
+
+ // Fetch schemas for each type
+ for (const group of groupResponse.group || []) {
+ const groupName = group.metadata.name;
+ const catalog = group.catalog;
+ const type = CatalogToGroupType[catalog];
+
+ if (type) {
+ try {
+ const schemaResponse = await getAllTypesOfResourceList(type,
groupName);
+ if (!schemaResponse.error) {
+ const schemaList = schemaResponse[type === 'property' ?
'properties' : type] || [];
+ const schemaNames = schemaList.map((s) =>
s.metadata?.name).filter(Boolean);
+ schemas[type] = [...new Set([...schemas[type], ...schemaNames])];
+ }
+ } catch (e) {
+ console.error(`Failed to fetch ${type} schemas for group
${groupName}:`, e);
+ }
+
+ // TopN resources are in CATALOG_MEASURE groups
+ if (catalog === 'CATALOG_MEASURE') {
+ try {
+ const topnResponse = await getTopNAggregationList(groupName);
+ if (!topnResponse.error) {
+ const topnList = topnResponse.topNAggregation || [];
+ const topnNames = topnList.map((s) =>
s.metadata?.name).filter(Boolean);
+ schemas.topn = [...new Set([...schemas.topn, ...topnNames])];
+ }
+ } catch (e) {
+ console.error(`Failed to fetch topn schemas for group
${groupName}:`, e);
+ }
+ }
+ }
+ }
Review Comment:
The schema fetching logic loops through all groups and makes sequential API
calls for each group (lines 309-340). This can result in a significant number
of sequential HTTP requests, which may cause slow initial load times. Consider
batching these requests or using Promise.all() to fetch schemas for multiple
groups in parallel.
##########
ui/src/components/CodeMirror/bydbql-hint.js:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import CodeMirror from 'codemirror';
+
+// BydbQL keywords
+const BYDBQL_KEYWORDS = [
+ 'SELECT',
+ 'FROM',
+ 'WHERE',
+ 'ORDER BY',
+ 'LIMIT',
+ 'OFFSET',
+ 'AND',
+ 'OR',
+ 'NOT',
+ 'IN',
+ 'LIKE',
+ 'BETWEEN',
+ 'IS',
+ 'NULL',
+ 'TRUE',
+ 'FALSE',
+ 'AS',
+ 'DISTINCT',
+ 'GROUP BY',
+ 'HAVING',
+ 'TIME',
+ 'ASC',
+ 'DESC',
+ 'SHOW',
+ 'TOP',
+ 'UNION',
+ 'INTERSECT',
+ 'EXCEPT',
+ 'AGGREGATE BY',
+ 'WITH QUERY_TRACE',
+ 'LIMIT',
+ 'OFFSET',
+ 'ORDER BY',
+ 'ON',
+ 'STAGES',
+ 'WHERE',
+ 'GROUP',
+ 'BY',
+ 'IN',
+ 'NOT IN',
+ 'HAVING',
+ 'MATCH',
+ 'SUM',
+ 'MEAN',
+ 'AVG',
+ 'COUNT',
+ 'MAX',
+ 'MIN',
+ 'TAG',
+];
+
+// BydbQL entity types
+const ENTITY_TYPES = ['STREAM', 'MEASURE', 'TRACE', 'PROPERTY', 'TOPN'];
+let schemasAndGroups = {
+ groups: [],
+ schemas: {},
+};
+
+export function updateSchemasAndGroups(groups, schemas) {
+ schemasAndGroups.groups = groups || [];
+ schemasAndGroups.schemas = schemas || {};
+}
+
+function getWordAt(cm, pos) {
+ const line = cm.getLine(pos.line);
+ const start = pos.ch;
+ const end = pos.ch;
+
+ let wordStart = start;
+ let wordEnd = end;
+
Review Comment:
The variables `start` and `end` are initialized with `pos.ch` but are never
used. Only `wordStart` and `wordEnd` are actually utilized in the function.
Consider removing these unused variable declarations.
```suggestion
let wordStart = pos.ch;
let wordEnd = pos.ch;
```
##########
ui/src/components/CodeMirror/index.vue:
##########
@@ -65,61 +67,127 @@
type: Boolean,
default: true,
},
+ enableHint: {
+ type: Boolean,
+ default: false,
+ },
+ extraKeys: {
+ type: Object,
+ default: () => ({}),
+ },
},
- emits: ['update:modelValue'],
+ emits: ['update:modelValue', 'ready'],
setup(props, { emit }) {
const textarea = ref(null);
const code = ref(props.modelValue);
let coder;
watch(
() => props.modelValue,
(val) => {
- coder?.setValue(val);
+ const currentValue = coder?.getValue();
+ if (val !== currentValue) {
+ coder?.setValue(val);
+ }
},
);
+
+ // Get mode based on prop
+ const getModeString = () => {
+ if (props.mode === 'bydbql') {
+ return 'text/x-bydbql';
+ }
+ if (props.mode === 'yaml') {
+ return 'text/x-yaml';
+ }
+ if (props.mode === 'css') {
+ return 'text/css';
+ }
+ return 'text/x-yaml';
+ };
+
const options = {
- mode: 'text/x-yaml',
+ mode: getModeString(),
tabSize: 2,
theme: props.theme,
lineNumbers: true,
line: true,
readOnly: props.readonly,
lint: props.lint,
- gutters: ['CodeMirror-lint-markers'],
+ gutters: props.lint ? ['CodeMirror-lint-markers'] : [],
styleActiveLine: props.styleActiveLine,
autoRefresh: props.autoRefresh,
height: '500px',
+ extraKeys: props.extraKeys,
};
+
const initialize = async () => {
try {
- /* let theme = `codemirror/theme/${props.theme}.css`
- await import(theme) */
if (props.lint) {
await import('codemirror/addon/lint/lint.js');
await import('codemirror/addon/lint/lint.css');
}
- /* if (props.mode) {
- await import(`codemirror/mode/${props.mode}/${props.mode}.js`)
- } */
if (props.autoRefresh) {
await import('codemirror/addon/display/autorefresh');
}
if (props.styleActiveLine) {
await import('codemirror/addon/selection/active-line');
}
- } catch (e) {}
+ if (props.enableHint) {
+ await import('codemirror/addon/hint/show-hint.js');
+ await import('codemirror/addon/hint/show-hint.css');
+ }
+ } catch (e) {
+ console.error('Error loading CodeMirror addons:', e);
+ }
+
coder = CodeMirror.fromTextArea(textarea.value, options);
+
coder.on('blur', (coder) => {
const newValue = coder.getValue();
emit('update:modelValue', newValue);
});
+
+ // Enable autocomplete on Ctrl+Space or when typing
+ if (props.enableHint) {
+ coder.on('keyup', (cm, event) => {
+ // Don't show hints for special keys
+ const excludedKeys = [
+ 8, // Backspace
+ 9, // Tab
+ 13, // Enter
+ 16,
+ 17,
+ 18, // Shift, Ctrl, Alt
+ 20, // Caps Lock
+ 27, // Escape
+ 33,
+ 34,
+ 35,
+ 36,
+ 37,
+ 38,
+ 39,
+ 40, // Page/Arrow keys
+ ];
+
+ if (!cm.state.completionActive &&
!excludedKeys.includes(event.keyCode)) {
+ CodeMirror.commands.autocomplete(cm, null, { completeSingle:
false });
Review Comment:
The `CodeMirror.commands.autocomplete` is called without specifying a hint
function or mode. CodeMirror will attempt to automatically determine which hint
helper to use based on the editor's mode. However, this might fail if the
mode-to-hint mapping isn't properly established. Consider explicitly passing
the hint helper: `CodeMirror.commands.autocomplete(cm, CodeMirror.hint.bydbql,
{ completeSingle: false })`.
##########
ui/src/components/BydbQL/Index.vue:
##########
@@ -273,19 +274,79 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`);
error.value = null;
executionTime.value = 0;
}
- // Setup keyboard shortcuts for CodeMirror
- onMounted(() => {
- nextTick(() => {
- // Find the CodeMirror instance and add keyboard shortcuts
- const codeMirrorElement = document.querySelector('.query-input
.CodeMirror');
- if (codeMirrorElement && codeMirrorElement.CodeMirror) {
- const cm = codeMirrorElement.CodeMirror;
- cm.setOption('extraKeys', {
- 'Ctrl-Enter': executeQuery,
- 'Cmd-Enter': executeQuery,
- });
- }
+
+ function onCodeMirrorReady(cm) {
+ codeMirrorInstance.value = cm;
+ cm.setOption('extraKeys', {
+ 'Ctrl-Enter': executeQuery,
+ 'Cmd-Enter': executeQuery,
+ 'Ctrl-Space': 'autocomplete',
});
Review Comment:
The `extraKeys` prop is passed during CodeMirror initialization (line 120),
but then overridden using `setOption` in the `onCodeMirrorReady` callback
(lines 280-284). This means any extraKeys passed via props will be completely
replaced rather than merged. If the parent component passes extraKeys through
props, they will be lost. Consider merging the prop extraKeys with the new keys
instead of replacing them entirely.
##########
ui/src/components/CodeMirror/index.vue:
##########
@@ -65,61 +67,127 @@
type: Boolean,
default: true,
},
+ enableHint: {
+ type: Boolean,
+ default: false,
+ },
+ extraKeys: {
+ type: Object,
+ default: () => ({}),
+ },
},
- emits: ['update:modelValue'],
+ emits: ['update:modelValue', 'ready'],
setup(props, { emit }) {
const textarea = ref(null);
const code = ref(props.modelValue);
let coder;
watch(
() => props.modelValue,
(val) => {
- coder?.setValue(val);
+ const currentValue = coder?.getValue();
+ if (val !== currentValue) {
+ coder?.setValue(val);
+ }
},
);
+
+ // Get mode based on prop
+ const getModeString = () => {
+ if (props.mode === 'bydbql') {
+ return 'text/x-bydbql';
+ }
+ if (props.mode === 'yaml') {
+ return 'text/x-yaml';
+ }
+ if (props.mode === 'css') {
+ return 'text/css';
+ }
+ return 'text/x-yaml';
+ };
+
const options = {
- mode: 'text/x-yaml',
+ mode: getModeString(),
tabSize: 2,
theme: props.theme,
lineNumbers: true,
line: true,
readOnly: props.readonly,
lint: props.lint,
- gutters: ['CodeMirror-lint-markers'],
+ gutters: props.lint ? ['CodeMirror-lint-markers'] : [],
styleActiveLine: props.styleActiveLine,
autoRefresh: props.autoRefresh,
height: '500px',
+ extraKeys: props.extraKeys,
};
+
const initialize = async () => {
try {
- /* let theme = `codemirror/theme/${props.theme}.css`
- await import(theme) */
if (props.lint) {
await import('codemirror/addon/lint/lint.js');
await import('codemirror/addon/lint/lint.css');
}
- /* if (props.mode) {
- await import(`codemirror/mode/${props.mode}/${props.mode}.js`)
- } */
if (props.autoRefresh) {
await import('codemirror/addon/display/autorefresh');
}
if (props.styleActiveLine) {
await import('codemirror/addon/selection/active-line');
}
- } catch (e) {}
+ if (props.enableHint) {
+ await import('codemirror/addon/hint/show-hint.js');
+ await import('codemirror/addon/hint/show-hint.css');
+ }
+ } catch (e) {
+ console.error('Error loading CodeMirror addons:', e);
+ }
Review Comment:
The CodeMirror initialization continues even if addon imports fail. If the
hint addon fails to load but `props.enableHint` is true, the subsequent
hint-related code (lines 151-177) will fail when attempting to use
`CodeMirror.commands.autocomplete`. Consider checking whether the hint addon
loaded successfully before setting up the autocomplete functionality, or
re-throw critical errors.
##########
ui/src/components/CodeMirror/bydbql-hint.js:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import CodeMirror from 'codemirror';
+
+// BydbQL keywords
+const BYDBQL_KEYWORDS = [
+ 'SELECT',
+ 'FROM',
+ 'WHERE',
+ 'ORDER BY',
+ 'LIMIT',
+ 'OFFSET',
+ 'AND',
+ 'OR',
+ 'NOT',
+ 'IN',
+ 'LIKE',
+ 'BETWEEN',
+ 'IS',
+ 'NULL',
+ 'TRUE',
+ 'FALSE',
+ 'AS',
+ 'DISTINCT',
+ 'GROUP BY',
+ 'HAVING',
+ 'TIME',
+ 'ASC',
+ 'DESC',
+ 'SHOW',
+ 'TOP',
+ 'UNION',
+ 'INTERSECT',
+ 'EXCEPT',
+ 'AGGREGATE BY',
+ 'WITH QUERY_TRACE',
+ 'LIMIT',
+ 'OFFSET',
+ 'ORDER BY',
+ 'ON',
+ 'STAGES',
+ 'WHERE',
+ 'GROUP',
+ 'BY',
+ 'IN',
+ 'NOT IN',
+ 'HAVING',
+ 'MATCH',
+ 'SUM',
+ 'MEAN',
+ 'AVG',
+ 'COUNT',
+ 'MAX',
+ 'MIN',
+ 'TAG',
+];
Review Comment:
Duplicate keyword entries detected in the BYDBQL_KEYWORDS array. The
following keywords appear multiple times:
- 'LIMIT' (lines 28, 54)
- 'OFFSET' (lines 29, 55)
- 'ORDER BY' (lines 27, 56)
- 'WHERE' (lines 26, 59)
- 'IN' (lines 33, 62)
- 'HAVING' (lines 43, 64)
These duplicates are unnecessary and should be removed to maintain code
clarity.
##########
ui/src/components/BydbQL/Index.vue:
##########
@@ -273,19 +274,79 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`);
error.value = null;
executionTime.value = 0;
}
- // Setup keyboard shortcuts for CodeMirror
- onMounted(() => {
- nextTick(() => {
- // Find the CodeMirror instance and add keyboard shortcuts
- const codeMirrorElement = document.querySelector('.query-input
.CodeMirror');
- if (codeMirrorElement && codeMirrorElement.CodeMirror) {
- const cm = codeMirrorElement.CodeMirror;
- cm.setOption('extraKeys', {
- 'Ctrl-Enter': executeQuery,
- 'Cmd-Enter': executeQuery,
- });
- }
+
+ function onCodeMirrorReady(cm) {
+ codeMirrorInstance.value = cm;
+ cm.setOption('extraKeys', {
+ 'Ctrl-Enter': executeQuery,
+ 'Cmd-Enter': executeQuery,
+ 'Ctrl-Space': 'autocomplete',
Review Comment:
The 'Ctrl-Space' key binding for 'autocomplete' is redundant. The
autocomplete is already triggered automatically on keyup (lines 152-176) for
all non-excluded keys. Additionally, the CodeMirror autocomplete command is
typically invoked as a function, not as a string. Consider either removing this
binding or verifying that it's necessary for manual triggering.
```suggestion
```
##########
ui/src/components/CodeMirror/bydbql-hint.js:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import CodeMirror from 'codemirror';
+
+// BydbQL keywords
+const BYDBQL_KEYWORDS = [
+ 'SELECT',
+ 'FROM',
+ 'WHERE',
+ 'ORDER BY',
+ 'LIMIT',
+ 'OFFSET',
+ 'AND',
+ 'OR',
+ 'NOT',
+ 'IN',
+ 'LIKE',
+ 'BETWEEN',
+ 'IS',
+ 'NULL',
+ 'TRUE',
+ 'FALSE',
+ 'AS',
+ 'DISTINCT',
+ 'GROUP BY',
+ 'HAVING',
+ 'TIME',
+ 'ASC',
+ 'DESC',
+ 'SHOW',
+ 'TOP',
+ 'UNION',
+ 'INTERSECT',
+ 'EXCEPT',
+ 'AGGREGATE BY',
+ 'WITH QUERY_TRACE',
+ 'LIMIT',
+ 'OFFSET',
+ 'ORDER BY',
+ 'ON',
+ 'STAGES',
+ 'WHERE',
+ 'GROUP',
+ 'BY',
+ 'IN',
+ 'NOT IN',
+ 'HAVING',
+ 'MATCH',
+ 'SUM',
+ 'MEAN',
+ 'AVG',
+ 'COUNT',
+ 'MAX',
+ 'MIN',
+ 'TAG',
+];
+
+// BydbQL entity types
+const ENTITY_TYPES = ['STREAM', 'MEASURE', 'TRACE', 'PROPERTY', 'TOPN'];
+let schemasAndGroups = {
+ groups: [],
+ schemas: {},
+};
+
+export function updateSchemasAndGroups(groups, schemas) {
+ schemasAndGroups.groups = groups || [];
+ schemasAndGroups.schemas = schemas || {};
+}
+
+function getWordAt(cm, pos) {
+ const line = cm.getLine(pos.line);
+ const start = pos.ch;
+ const end = pos.ch;
+
+ let wordStart = start;
+ let wordEnd = end;
+
+ // Find word boundaries
+ while (wordStart > 0 && /\w/.test(line.charAt(wordStart - 1))) {
+ wordStart--;
+ }
+ while (wordEnd < line.length && /\w/.test(line.charAt(wordEnd))) {
+ wordEnd++;
+ }
+
+ return {
+ word: line.slice(wordStart, wordEnd),
+ start: wordStart,
+ end: wordEnd,
+ };
+}
+
+// Analyze the query context to determine what to suggest
+function getQueryContext(cm, cursor) {
+ const textBeforeCursor = cm.getRange({ line: 0, ch: 0 }, cursor);
+
+ // Check if we're typing after 'in' (group name context)
+ const inMatch =
textBeforeCursor.match(/\bFROM\s+(STREAM|MEASURE|TRACE|PROPERTY|TOPN)\s+(\w+)\s+in\s+\w*$/i);
+ if (inMatch) {
+ return { type: 'group', entityType: inMatch[1].toLowerCase() };
+ }
+
+ // Check if we're typing after schema name (expecting 'in')
+ const schemaMatch =
textBeforeCursor.match(/\bFROM\s+(STREAM|MEASURE|TRACE|PROPERTY|TOPN)\s+(\w+)\s+\w*$/i);
+ if (schemaMatch) {
+ return { type: 'in_keyword' };
+ }
+
+ // Check if we're typing after entity type (schema name context)
+ const entityMatch =
textBeforeCursor.match(/\bFROM\s+(STREAM|MEASURE|TRACE|PROPERTY|TOPN)\s+\w*$/i);
+ if (entityMatch) {
+ return { type: 'schema', entityType: entityMatch[1].toLowerCase() };
+ }
+
+ // Check if we're typing after FROM (entity type context)
+ if (/\bFROM\s+\w*$/i.test(textBeforeCursor)) {
Review Comment:
The regex patterns for matching context (lines 115, 121, 127, 133) use
`\w*$` which will match word characters at the end. However, if the cursor is
positioned in the middle of a word or immediately after whitespace, these
patterns may not behave as expected. The pattern assumes the user is always
typing at the very end of the text, which might not account for multi-line
queries or editing in the middle of the query.
```suggestion
const inMatch =
textBeforeCursor.match(/\bFROM\s+(STREAM|MEASURE|TRACE|PROPERTY|TOPN)\s+(\w+)\s+in\s+(\w*)$/i);
if (inMatch) {
return { type: 'group', entityType: inMatch[1].toLowerCase() };
}
// Check if we're typing after schema name (expecting 'in')
const schemaMatch =
textBeforeCursor.match(/\bFROM\s+(STREAM|MEASURE|TRACE|PROPERTY|TOPN)\s+(\w+)\s+(\w*)$/i);
if (schemaMatch) {
return { type: 'in_keyword' };
}
// Check if we're typing after entity type (schema name context)
const entityMatch =
textBeforeCursor.match(/\bFROM\s+(STREAM|MEASURE|TRACE|PROPERTY|TOPN)\s+(\w*)$/i);
if (entityMatch) {
return { type: 'schema', entityType: entityMatch[1].toLowerCase() };
}
// Check if we're typing after FROM (entity type context)
if (/\bFROM\s+(\w*)$/i.test(textBeforeCursor)) {
```
##########
ui/src/components/BydbQL/Index.vue:
##########
@@ -304,12 +365,14 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`);
<div class="query-input-container">
<CodeMirror
v-model="queryText"
- :mode="'sql'"
- :lint="false"
+ :mode="'bydbql'"
+ :lint="true"
Review Comment:
Linting is enabled for bydbql mode (line 369), but there's no lint
implementation for the 'bydbql' mode. The CodeMirror lint addon requires a
mode-specific linter to be registered. Without a bydbql-specific linter,
enabling lint will likely have no effect or could cause errors. Consider either
implementing a bydbql linter or setting `:lint="false"` for bydbql mode.
```suggestion
:lint="false"
```
##########
ui/src/components/CodeMirror/bydbql-mode.js:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import CodeMirror from 'codemirror';
+
+// Define BydbQL mode extending SQL
+CodeMirror.defineMode('bydbql', function (config) {
+ const sqlMode = CodeMirror.getMode(config, 'text/x-sql');
+
+ const entityTypes = {
+ STREAM: true,
+ MEASURE: true,
+ TRACE: true,
+ PROPERTY: true,
+ TOPN: true,
+ };
+
+ // BydbQL-specific keywords
+ const bydbqlKeywords = {
+ SELECT: true,
+ FROM: true,
+ WHERE: true,
+ ORDER: true,
+ BY: true,
+ ASC: true,
+ DESC: true,
+ LIMIT: true,
+ OFFSET: true,
+ AND: true,
+ OR: true,
+ NOT: true,
Review Comment:
This property is duplicated [in a later property](1).
##########
ui/src/components/CodeMirror/index.vue:
##########
@@ -65,61 +67,127 @@
type: Boolean,
default: true,
},
+ enableHint: {
+ type: Boolean,
+ default: false,
+ },
+ extraKeys: {
+ type: Object,
+ default: () => ({}),
+ },
},
- emits: ['update:modelValue'],
+ emits: ['update:modelValue', 'ready'],
setup(props, { emit }) {
const textarea = ref(null);
const code = ref(props.modelValue);
let coder;
watch(
() => props.modelValue,
(val) => {
- coder?.setValue(val);
+ const currentValue = coder?.getValue();
+ if (val !== currentValue) {
+ coder?.setValue(val);
+ }
},
);
+
+ // Get mode based on prop
+ const getModeString = () => {
+ if (props.mode === 'bydbql') {
+ return 'text/x-bydbql';
+ }
+ if (props.mode === 'yaml') {
+ return 'text/x-yaml';
+ }
+ if (props.mode === 'css') {
+ return 'text/css';
+ }
+ return 'text/x-yaml';
+ };
+
const options = {
- mode: 'text/x-yaml',
+ mode: getModeString(),
tabSize: 2,
theme: props.theme,
lineNumbers: true,
line: true,
readOnly: props.readonly,
lint: props.lint,
- gutters: ['CodeMirror-lint-markers'],
+ gutters: props.lint ? ['CodeMirror-lint-markers'] : [],
styleActiveLine: props.styleActiveLine,
autoRefresh: props.autoRefresh,
height: '500px',
+ extraKeys: props.extraKeys,
};
+
const initialize = async () => {
try {
- /* let theme = `codemirror/theme/${props.theme}.css`
- await import(theme) */
if (props.lint) {
await import('codemirror/addon/lint/lint.js');
await import('codemirror/addon/lint/lint.css');
}
- /* if (props.mode) {
- await import(`codemirror/mode/${props.mode}/${props.mode}.js`)
- } */
if (props.autoRefresh) {
await import('codemirror/addon/display/autorefresh');
}
if (props.styleActiveLine) {
await import('codemirror/addon/selection/active-line');
}
- } catch (e) {}
+ if (props.enableHint) {
+ await import('codemirror/addon/hint/show-hint.js');
+ await import('codemirror/addon/hint/show-hint.css');
+ }
+ } catch (e) {
+ console.error('Error loading CodeMirror addons:', e);
+ }
+
coder = CodeMirror.fromTextArea(textarea.value, options);
+
coder.on('blur', (coder) => {
const newValue = coder.getValue();
emit('update:modelValue', newValue);
});
+
+ // Enable autocomplete on Ctrl+Space or when typing
Review Comment:
The comment states "Enable autocomplete on Ctrl+Space or when typing", but
the Ctrl+Space binding is set separately in the parent component
(BydbQL/Index.vue line 283). The comment is misleading as this code block only
handles the "when typing" part. Consider updating the comment to accurately
reflect that this code handles automatic triggering on keyup events.
```suggestion
// Enable automatic autocomplete when typing (on keyup events)
```
##########
ui/src/components/CodeMirror/index.vue:
##########
@@ -65,61 +67,127 @@
type: Boolean,
default: true,
},
+ enableHint: {
+ type: Boolean,
+ default: false,
+ },
+ extraKeys: {
+ type: Object,
+ default: () => ({}),
+ },
},
- emits: ['update:modelValue'],
+ emits: ['update:modelValue', 'ready'],
setup(props, { emit }) {
const textarea = ref(null);
const code = ref(props.modelValue);
let coder;
watch(
() => props.modelValue,
(val) => {
- coder?.setValue(val);
+ const currentValue = coder?.getValue();
+ if (val !== currentValue) {
+ coder?.setValue(val);
+ }
},
);
+
+ // Get mode based on prop
+ const getModeString = () => {
+ if (props.mode === 'bydbql') {
+ return 'text/x-bydbql';
+ }
+ if (props.mode === 'yaml') {
+ return 'text/x-yaml';
+ }
+ if (props.mode === 'css') {
+ return 'text/css';
+ }
+ return 'text/x-yaml';
+ };
+
const options = {
- mode: 'text/x-yaml',
+ mode: getModeString(),
tabSize: 2,
theme: props.theme,
lineNumbers: true,
line: true,
readOnly: props.readonly,
lint: props.lint,
- gutters: ['CodeMirror-lint-markers'],
+ gutters: props.lint ? ['CodeMirror-lint-markers'] : [],
styleActiveLine: props.styleActiveLine,
autoRefresh: props.autoRefresh,
height: '500px',
+ extraKeys: props.extraKeys,
};
+
const initialize = async () => {
try {
- /* let theme = `codemirror/theme/${props.theme}.css`
- await import(theme) */
if (props.lint) {
await import('codemirror/addon/lint/lint.js');
await import('codemirror/addon/lint/lint.css');
}
- /* if (props.mode) {
- await import(`codemirror/mode/${props.mode}/${props.mode}.js`)
- } */
if (props.autoRefresh) {
await import('codemirror/addon/display/autorefresh');
}
if (props.styleActiveLine) {
await import('codemirror/addon/selection/active-line');
}
- } catch (e) {}
+ if (props.enableHint) {
+ await import('codemirror/addon/hint/show-hint.js');
+ await import('codemirror/addon/hint/show-hint.css');
+ }
+ } catch (e) {
+ console.error('Error loading CodeMirror addons:', e);
+ }
+
coder = CodeMirror.fromTextArea(textarea.value, options);
+
coder.on('blur', (coder) => {
const newValue = coder.getValue();
emit('update:modelValue', newValue);
});
+
+ // Enable autocomplete on Ctrl+Space or when typing
+ if (props.enableHint) {
+ coder.on('keyup', (cm, event) => {
+ // Don't show hints for special keys
+ const excludedKeys = [
+ 8, // Backspace
+ 9, // Tab
+ 13, // Enter
+ 16,
+ 17,
+ 18, // Shift, Ctrl, Alt
+ 20, // Caps Lock
+ 27, // Escape
+ 33,
+ 34,
+ 35,
+ 36,
+ 37,
+ 38,
+ 39,
+ 40, // Page/Arrow keys
+ ];
+
+ if (!cm.state.completionActive &&
!excludedKeys.includes(event.keyCode)) {
+ CodeMirror.commands.autocomplete(cm, null, { completeSingle:
false });
+ }
+ });
Review Comment:
The autocomplete is triggered on every keyup event that isn't in the
excluded list. This could cause performance issues with large datasets or slow
API responses, as hints will be generated and displayed on every character
typed. Consider adding debouncing or throttling to reduce the frequency of hint
generation, especially when `schemasAndGroups` contains large amounts of data.
##########
ui/src/components/CodeMirror/bydbql-mode.js:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import CodeMirror from 'codemirror';
+
+// Define BydbQL mode extending SQL
+CodeMirror.defineMode('bydbql', function (config) {
+ const sqlMode = CodeMirror.getMode(config, 'text/x-sql');
+
+ const entityTypes = {
+ STREAM: true,
+ MEASURE: true,
+ TRACE: true,
+ PROPERTY: true,
+ TOPN: true,
+ };
+
+ // BydbQL-specific keywords
+ const bydbqlKeywords = {
+ SELECT: true,
+ FROM: true,
+ WHERE: true,
+ ORDER: true,
+ BY: true,
+ ASC: true,
+ DESC: true,
+ LIMIT: true,
+ OFFSET: true,
+ AND: true,
+ OR: true,
+ NOT: true,
+ IN: true,
+ LIKE: true,
+ BETWEEN: true,
+ IS: true,
+ NULL: true,
+ TRUE: true,
+ FALSE: true,
+ AS: true,
+ DISTINCT: true,
+ ALL: true,
+ ANY: true,
+ SOME: true,
+ EXISTS: true,
+ CASE: true,
+ WHEN: true,
+ THEN: true,
+ ELSE: true,
+ END: true,
+ UNION: true,
+ INTERSECT: true,
+ EXCEPT: true,
+ GROUP: true,
+ HAVING: true,
+ TIME: true,
+ SHOW: true,
+ TOP: true,
+ AGGREGATE: true,
+ BY: true,
+ IN: true,
+ NOT: true,
+ MATCH: true,
+ SUM: true,
+ MEAN: true,
+ AVG: true,
+ COUNT: true,
+ MAX: true,
+ MIN: true,
+ TAG: true,
+ };
Review Comment:
Duplicate keyword entries detected. The keywords 'BY', 'IN', and 'NOT' are
defined multiple times in the object (lines 40, 75, 76, and lines 48, 76, 77).
In JavaScript objects, duplicate keys will result in only the last value being
kept, but this creates confusion and is a maintainability issue.
##########
ui/src/components/CodeMirror/bydbql-mode.js:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import CodeMirror from 'codemirror';
+
+// Define BydbQL mode extending SQL
+CodeMirror.defineMode('bydbql', function (config) {
+ const sqlMode = CodeMirror.getMode(config, 'text/x-sql');
+
+ const entityTypes = {
+ STREAM: true,
+ MEASURE: true,
+ TRACE: true,
+ PROPERTY: true,
+ TOPN: true,
+ };
+
+ // BydbQL-specific keywords
+ const bydbqlKeywords = {
+ SELECT: true,
+ FROM: true,
+ WHERE: true,
+ ORDER: true,
+ BY: true,
+ ASC: true,
+ DESC: true,
+ LIMIT: true,
+ OFFSET: true,
+ AND: true,
+ OR: true,
+ NOT: true,
+ IN: true,
Review Comment:
This property is duplicated [in a later property](1).
##########
ui/src/components/CodeMirror/bydbql-mode.js:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import CodeMirror from 'codemirror';
+
+// Define BydbQL mode extending SQL
+CodeMirror.defineMode('bydbql', function (config) {
+ const sqlMode = CodeMirror.getMode(config, 'text/x-sql');
Review Comment:
The bydbql mode depends on the SQL mode ('text/x-sql') being available, but
there's no explicit check to ensure the SQL mode is loaded before defining the
bydbql mode. If the SQL mode import (line 31 in index.vue) fails or loads after
this file, `CodeMirror.getMode(config, 'text/x-sql')` could return an undefined
or default mode, causing the bydbql mode to not work correctly.
```suggestion
const sqlMode = CodeMirror.getMode(config, 'text/x-sql');
if (!sqlMode || typeof sqlMode.token !== 'function') {
throw new Error(
"CodeMirror SQL mode ('text/x-sql') must be loaded before defining the
BydbQL mode. Please ensure the SQL mode is imported first."
);
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
