Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Leave aliases to be unquoted in autocomplete #1740

Merged
merged 6 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 3 additions & 20 deletions apps/studio/src/components/TabQueryEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@
import 'codemirror/addon/search/matchesonscrollbar'
import 'codemirror/addon/search/matchesonscrollbar.css'
import 'codemirror/addon/search/searchcursor'
import { registerAutoquote } from '@/lib/codemirror'

import setKeybindingsFromVimrc from "../lib/readVimrc"

Expand Down Expand Up @@ -745,6 +746,7 @@
'mariadb': 'text/x-mariadb',
'sqlite': 'text/x-sqlite',
'cassandra': 'text/x-cassandra',
'redshift': 'text/x-pgsql',
};

const extraKeys = {}
Expand Down Expand Up @@ -809,26 +811,7 @@
this.unsavedText = cm.getValue()
})

if (this.connectionType === 'postgresql') {
this.editor.on("beforeChange", (_cm, co) => {
const { to, from, origin, text } = co;

// eslint-disable-next-line
// @ts-ignore
const keywords = CodeMirror.resolveMode(this.editor.options.mode).keywords

// quote names when needed
if (origin === 'complete' && keywords[text[0].toLowerCase()] != true) {
const names = text[0]
.match(/("[^"]*"|[^.]+)/g)
.map(n => /^\d/.test(n) ? `"${n}"` : n)
.map(n => /[^a-z0-9_]/.test(n) && !/"/.test(n) ? `"${n}"` : n)
.join('.')

co.update(from, to, [names], origin)
}
})
}
registerAutoquote(this.editor)

// TODO: make this not suck
this.editor.on('keyup', this.maybeAutoComplete)
Expand Down
14 changes: 14 additions & 0 deletions apps/studio/src/lib/codemirror-definition.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import CodeMirror from "codemirror";

CodeMirror.defineMIME("text/x-pgsql", {
// eslint-disable-next-line
// @ts-ignore
...CodeMirror.resolveMode("text/x-pgsql"),
identifierQuote: '"',
hooks: {
// eslint-disable-next-line
// @ts-ignore
'"': CodeMirror.resolveMode("text/x-sqlite").hooks['"'],
},
});

53 changes: 46 additions & 7 deletions apps/studio/src/lib/codemirror.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,52 @@
import CodeMirror from "codemirror";

CodeMirror.defineMIME("text/x-pgsql", {
export function autoquoteHandler(
instance: CodeMirror.Editor,
changeObj: CodeMirror.EditorChangeCancellable
) {
if (!instance.getOption("mode").includes("pgsql")) return;
azmy60 marked this conversation as resolved.
Show resolved Hide resolved

// eslint-disable-next-line
// @ts-ignore
const { to, from, origin, text } = changeObj;

// eslint-disable-next-line
// @ts-ignore
...CodeMirror.resolveMode("text/x-pgsql"),
identifierQuote: '"',
hooks: {
const keywords = CodeMirror.resolveMode(instance.getOption("mode")).keywords;

// quote names when needed
if (origin === "complete" && keywords[text[0].toLowerCase()] != true) {
// eslint-disable-next-line
// @ts-ignore
'"': CodeMirror.resolveMode("text/x-sqlite").hooks['"'],
},
});
const alias = instance.activeAlias;
const names = text[0]
.match(/("[^"]*"|[^.]+)/g)
.map((n) => (/^\d/.test(n) && n !== alias ? `"${n}"` : n))
.map((n) =>
/[^a-z0-9_]/.test(n) && !/"/.test(n) && n !== alias ? `"${n}"` : n
)
.join(".");

changeObj.update(from, to, [names]);
}
}

export function registerAutoquote(
instance: CodeMirror.Editor,
onBeforeChange?: (
instance: CodeMirror.Editor,
changeObj: CodeMirror.EditorChangeCancellable
) => void
) {
instance.on("beforeChange", onBeforeChange ?? autoquoteHandler);
}

export function unregisterAutoquote(
instance: CodeMirror.Editor,
onBeforeChange?: (
instance: CodeMirror.Editor,
changeObj: CodeMirror.EditorChangeCancellable
) => void
) {
instance.off("beforeChange", onBeforeChange ?? autoquoteHandler);
}
2 changes: 1 addition & 1 deletion apps/studio/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import 'codemirror/mode/sql/sql'
import 'codemirror/mode/diff/diff'
import './vendor/sql-hint'
import './vendor/show-hint'
import './lib/codemirror'
import './lib/codemirror-definition'

import store from './store/index'
import 'reflect-metadata'
Expand Down
10 changes: 10 additions & 0 deletions apps/studio/src/vendor/sql-hint/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@
if (table !== oldTable) alias = true;
}

if (alias) editor.activeAlias = aliasTable;
else editor.activeAlias = undefined;

var columns = editor.options.getColumns ? await editor.options.getColumns(table): getTable(table)

if (columns && columns.columns)
Expand Down Expand Up @@ -257,6 +260,13 @@
if (defaultTable.columns)
defaultTable = defaultTable.columns;

/*
* Getting info of which alias is in autocomplete.
* Useful for leaving alias to be unquoted in postgres
* which is handled outside of this module
*/
editor.activeAlias = undefined;

var cur = editor.getCursor();
var result = [];
var token = editor.getTokenAt(cur), start, end, search;
Expand Down
3 changes: 2 additions & 1 deletion apps/studio/tests/unit/codemirror/completions.spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// "forked" from CodeMirror, copyright (c) by Marijn Haverbeke and others
// Distributed under an MIT license: https://codemirror.net/5/LICENSE

import { Pos, testCompletions as test } from "./helpers";
import { Pos } from "codemirror";
import { testCompletions as test } from "./helpers";

const simpleTables = {
users: ["name", "score", "birthDate"],
Expand Down
79 changes: 69 additions & 10 deletions apps/studio/tests/unit/codemirror/helpers.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,92 @@
import CodeMirror from "codemirror";
import "@/vendor/sql-hint/index";
import "@/lib/codemirror"
import "@/lib/codemirror-definition"
import {
registerAutoquote,
unregisterAutoquote,
autoquoteHandler,
} from "@/lib/codemirror";

export const Pos = CodeMirror.Pos;
class Editor {
constructor(opts = {}) {
this.textarea = document.createElement("textarea");
document.body.appendChild(this.textarea);

this.cm = CodeMirror.fromTextArea(this.textarea, {
...opts,
mode: opts.mode || "text/x-mysql",
});

this.cm.setValue(opts.value || "");
}

async complete(completeTo) {
function mockedBeforeChange(cm, co) {
co.origin = "complete";
autoquoteHandler(cm, co);
}

registerAutoquote(this.cm, mockedBeforeChange);

const hint = await this.hint();

this.cm.replaceRange(completeTo, hint.from, hint.to);

unregisterAutoquote(this.cm, mockedBeforeChange);
}

async hint() {
return await CodeMirror.hint.sql(this.cm, {
tables: this.cm.getOption("tables"),
defaultTable: this.cm.getOption("defaultTable"),
disableKeywords: this.cm.getOption("disableKeywords"),
});
}

destroy() {
document.body.removeChild(this.textarea);
}
}

function _testCompletions(it, name, spec) {
it(name, async () => {
const textarea = document.createElement("textarea");
document.body.appendChild(textarea);
const cm = CodeMirror.fromTextArea(textarea, {
const editor = new Editor({
mode: spec.mode || "text/x-mysql",
value: spec.value,
getColumns: spec.getColumns,
});
cm.setValue(spec.value);
cm.setCursor(spec.cursor);
const completions = await CodeMirror.hint.sql(cm, {
tables: spec.tables,
defaultTable: spec.defaultTable,
disableKeywords: spec.disableKeywords,
});
editor.cm.setCursor(spec.cursor);
const completions = await editor.hint();
expect(spec.list.sort()).toEqual(completions.list.sort());
expect(spec.from).toEqual(completions.from);
expect(spec.to).toEqual(completions.to);
document.body.removeChild(textarea);
editor.destroy();
});
}

function _testAutoquotes(it, name, spec) {
it(name, async () => {
const editor = new Editor({
mode: "text/x-pgsql",
tables: spec.tables,
value: spec.value,
});
editor.cm.setCursor(spec.cursor);
await editor.complete(spec.completeTo);
expect(editor.cm.getValue()).toBe(spec.result);
editor.destroy();
});
}

/** @type jest.It */
export const testCompletions = _testCompletions.bind(test, test);
testCompletions['only'] = _testCompletions.bind(null, test['only']);
testCompletions['skip'] = _testCompletions.bind(null, test['skip']);

/** @type jest.It */
export const testAutoquotes = _testAutoquotes.bind(test, test);
testAutoquotes["only"] = _testAutoquotes.bind(null, test["only"]);
testAutoquotes["skip"] = _testAutoquotes.bind(null, test["skip"]);
36 changes: 36 additions & 0 deletions apps/studio/tests/unit/codemirror/quotes.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { Pos } from "codemirror";
import { testAutoquotes as test } from "./helpers";

describe("Codemirror autoquotes", () => {
test("Lowercased identifier", {
tables: { Foobar: [] },
value: "SELECT * FROM Foo",
cursor: Pos(0, 17),
completeTo: "foobar",
result: "SELECT * FROM foobar",
});

test("Lowercased identifier, uppercased alias", {
tables: { foo: ["baz"] },
value: "SELECT Bar. FROM foo AS Bar",
cursor: Pos(0, 11),
completeTo: "Bar.baz",
result: "SELECT Bar.baz FROM foo AS Bar",
});

test("Uppercased identifier", {
tables: { Foobar: [] },
value: "SELECT * FROM Foo",
cursor: Pos(0, 17),
completeTo: "Foobar",
result: 'SELECT * FROM "Foobar"',
});

test("Uppercased identifier, uppercased alias", {
tables: { foo: ["Baz"] },
value: "SELECT Bar. FROM foo AS Bar",
cursor: Pos(0, 11),
completeTo: "Bar.Baz",
result: 'SELECT Bar."Baz" FROM foo AS Bar',
});
});