Issue #1883 + Typescript Plugin Improvements #1986
Conversation
|
I love it. Thank you! |
src/56sprintf.js
Outdated
| }); | ||
| }; | ||
|
|
||
| // Regular expression updated with comments for readability |
src/56sprintf.js
Outdated
|
|
||
| stdfn.SPRINTF.H = (ins, x) => { | ||
| let cleanIns = String(ins).replace(/,/g, ''); // Remove commas for number parsing | ||
| x[2] = x[2] === '-' ? '+' : '-'; // Adjust alignment for string output |
There was a problem hiding this comment.
Are you sure that comment is correct?
src/63createvertex.js
Outdated
| @@ -1,21 +1,18 @@ | |||
| /* | |||
| // | |||
| // CREATE VERTEX for AlaSQL | |||
| // Date: 21.04.2015 | |||
| // (c) 2015, Andrey Gershun | |||
| // Date: 04/11/2024 | |||
There was a problem hiding this comment.
Please do not change the date.
src/63createvertex.js
Outdated
| // Date: 21.04.2015 | ||
| // (c) 2015, Andrey Gershun | ||
| // Date: 04/11/2024 | ||
| // (c) 2015, Andrey Gershu |
There was a problem hiding this comment.
Please dont change the surname of the author...
src/63createvertex.js
Outdated
| yy.CreateVertex = function (params) { | ||
| return Object.assign(this, params); | ||
| }; | ||
| yy.CreateVertex = (params) => Object.assign(this, params); // Updated to arrow function |
There was a problem hiding this comment.
Please dont leave comments like this as it will not make sense in the future
There was a problem hiding this comment.
Thanks for letting me know and sorry for the late reply
src/63createvertex.js
Outdated
| // todo: SET | ||
| // todo: CONTENT | ||
| // todo: SELECT | ||
| // Future TODOs: SET, CONTENT, SELECT |
There was a problem hiding this comment.
Please keep the original comments please
src/63createvertex.js
Outdated
| var s = 'this.queriesfn[' + (this.queriesidx - 1) + '](this.params,null,' + context + ')'; | ||
| return s; | ||
| yy.CreateEdge.prototype.toJS = (context) => { | ||
| return `this.queriesfn[${this.queriesidx - 1}](this.params,null,${context})`; // Template literal for consistency |
src/63createvertex.js
Outdated
|
|
||
| // Optional functions | ||
| namefn?.(edge); | ||
| namefn?.(edge); // Optional chaining |
src/63createvertex.js
Outdated
| @@ -196,7 +157,7 @@ yy.CreateGraph.prototype.execute = function (databaseid, params, cb) { | |||
| if (g.class !== undefined) e.$class = g.class; | |||
|
|
|||
| let db = alasql.databases[databaseid]; | |||
| e.$id = e.$id !== undefined ? e.$id : db.counter++; | |||
| e.$id = e.$id ?? db.counter++; | |||
src/63createvertex.js
Outdated
| } | ||
| } | ||
| return undefined; | ||
| return Object.values(alasql.databases[alasql.useid].objects).find((obj) => obj.name === name); |
There was a problem hiding this comment.
We simply cant be making copies of all the data in an object for operations like this. We need to keep the original or find a way where we can search the original object without making a copy.
src/63createvertex.js
Outdated
| if (typeof g.json !== 'undefined') { | ||
| extend(v, new Function('params,alasql', 'var y;return ' + g.json.toJS())(params, alasql)); | ||
| const db = alasql.databases[databaseid]; | ||
| const v = { |
There was a problem hiding this comment.
Too many changes. And I dont feel like this will provide the same result. where. is the .prop aspect?
src/63createvertex.js
Outdated
| res.push(v.$id); | ||
| return v; | ||
| } | ||
| }; | ||
| yy.CreateGraph.prototype.compile1 = function (databaseid) { |
There was a problem hiding this comment.
Did we just remove all of this?!?
src/76usedatabase.js
Outdated
| yy.CreateDatabase = function (params) { | ||
| return Object.assign(this, params); | ||
| }; | ||
| yy.CreateDatabase = (params) => Object.assign(this, params); // Updated to arrow function |
There was a problem hiding this comment.
Please keep regular functions for high level so they remain informative in the stack trace
src/76usedatabase.js
Outdated
| yy.CreateDatabase.prototype.toString = function () { | ||
| let s = 'CREATE '; // Ensure there's a space after CREATE | ||
| let s = 'CREATE '; // Added space after 'CREATE' for clarity |
src/76usedatabase.js
Outdated
| if (this.engineid) s += `${this.engineid} `; | ||
| s += 'DATABASE '; | ||
| if (this.ifnotexists) s += 'IF NOT EXISTS '; | ||
| s += `${this.databaseid} `; | ||
|
|
||
| if (this.args && this.args.length > 0) { | ||
| if (this.args?.length) { // Optional chaining for args presence check |
types/alasql.d.ts
Outdated
| * | ||
| * @type {tableLookUp} | ||
| * @memberof database | ||
| * Collection of tables in the database. |
types/alasql.d.ts
Outdated
| * | ||
| * @type {any[]} | ||
| * @memberof table | ||
| * Array of data rows within the table. |
types/alasql.d.ts
Outdated
| * | ||
| * @type {databaseLookUp} | ||
| * @memberof AlaSQL | ||
| * Dictionary of databases managed by AlaSQL. |
types/alasql.d.ts
Outdated
| * Equivalent to alasql('USE '+databaseid). This will change the current | ||
| * database to the one specified. This will update the useid property and | ||
| * the tables property. | ||
| * Switches the current database context. |
There was a problem hiding this comment.
Please keep the context of the old comment
types/alasql.d.ts
Outdated
| * | ||
| * @type {string} | ||
| * @memberof AlaSQL | ||
| * Current active database ID. Defaults to 'alasql' if none specified. |
|
Looks like the tests are not running. Please make sure to do a |
Charlesnorris509
force-pushed
the
develop
branch
from
01e3263 to
0e5e387
Compare
Key changes include using arrow functions and template literals for cleaner syntax, and optional chaining to streamline property access where values may be undefined. Destructured assignments enhance readability, and constants are used for immutability where applicable. Additionally, inline comments clarify each change for easy reference, while error handling improvements ensure the code remains robust and clear. These changes collectively modernize the codebase while preserving its original functionality and use cases