Show SQL cursors in outline view #216

sebCIL · 2023-03-06T13:03:26Z

Changes

Show SQL cursors in outline view.

I'm not sure about the icon.

Checklist

have tested my change
updated relevant documentation
Remove any/all console.logs I added
eslint is not complaining
have added myself to the contributors' list in the README
for feature PRs: PR only includes one feature enhancement.

sebCIL · 2023-03-06T14:26:00Z

Sorry, after more tests, I have some bug with fixed format.

worksofliam · 2023-03-06T15:09:56Z

@sebCIL please also write some test cases for this feature.

Thanks!

sebCIL · 2023-03-08T07:48:07Z

@sebCIL please also write some test cases for this feature.

Thanks!

Done.

worksofliam · 2023-03-19T19:56:32Z

@sebCIL When running these tests, I am getting an error from a test:

vscode-rpgle barry$ npm run test

> [email protected] test
> tsx tests

Running 157 tests:
/Users/barry/Repos/vscode-rpgle/tests/suite/linter.js:2429
  assert.strictEqual(errors.length, 1);
         ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

3 !== 1

    at null.exports.linter34 (/Users/barry/Repos/vscode-rpgle/tests/suite/linter.js:2429:10)
    at null.run (/Users/barry/Repos/vscode-rpgle/tests/index.js:17:5) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 3,
  expected: 1,
  operator: 'strictEqual'
}

sebCIL · 2023-03-20T12:17:08Z

Ok, I fixed it.

worksofliam · 2023-05-12T15:22:43Z

@chrjorgensen @sebjulliand Any chance I can get your opinion on this PR? Do you think showing the cursors is standard and would be useful to the user?

chrjorgensen · 2023-05-15T11:33:02Z

@worksofliam I have not seen cursors in an outline view before - it's not in RDi.
But to me it would be a nice addition - helping in locating the definition of a SQL cursor.

worksofliam

Overall a good implementation, but the EXEC handling logic needs formatting.

Also, I don't think we should be appending the CURSOR keyword on the definition. I think instead we should use a different SymbolKind to indicate the difference.

See my review notes - thanks!

worksofliam · 2023-05-15T15:32:05Z

server/src/language/models/cache.js

@@ -42,6 +42,10 @@ export default class Cache {

    /** @type {IncludeStatement[]} */
    this.includes = cache.includes || [];
+
+    /** @type {Declaration[]} */
+    this.cursor = cache.cursor || [];


Please use a plural here. cursor to be cursors to follow the rest of the properties in this class.

worksofliam · 2023-05-15T15:33:08Z

server/src/language/parser.js

+                indexCursorName = indexCursor-1;
+              }
+              if (currentCursor === undefined 
+               && indexDeclare >=0 


Indentation here is incorrect. eslint should be able to fix this up automatically for you. (or Format Document in VS Code)

worksofliam · 2023-05-15T15:33:26Z

server/src/language/parser.js

+                  path: file,
+                  line: statementStartingLine
+                };
+                currentCursor.keywords.push(`CURSOR`);


Cursor keyword should not be used here. See final review notes.

worksofliam · 2023-05-15T15:33:45Z

server/src/language/parser.js

+                indexCursorName = indexCursor-1;
+              }
+              if (currentCursor === undefined 
+               && indexDeclare >=0 


Indentation also incorrect here.

worksofliam · 2023-05-15T15:38:09Z

server/src/providers/documentSymbols.ts

+					.map(def => DocumentSymbol.create(
+						def.name,
+						def.keywords.join(` `).trim(),
+						SymbolKind.Constant,


Incorrect symbol kind. I think Operator or Interface

worksofliam · 2023-05-19T18:08:59Z

@sebCIL Review incoming today. Thank you!

worksofliam

Looks like there are some invalid references which causes the tests to fail. Check out my comments and request my review again when it's ready.

Thanks!

p.s. I am waiting for this PR to be merged before merging a rather large cleanup PR. Thank you!

worksofliam · 2023-05-19T18:35:58Z

server/src/language/models/cache.js

@@ -120,6 +127,7 @@ export default class Cache {
      ...this.subroutines.filter(def => def.name.toUpperCase() === name),
      ...this.variables.filter(def => def.name.toUpperCase() === name),
      ...this.indicators.filter(def => def.name.toUpperCase() === name),
+      ...this.cursor.filter(def => def.name.toUpperCase() === name),


Looks like you've updated the cursors property but missed renaming it elsewhere.

worksofliam · 2023-05-19T18:36:10Z

server/src/language/models/cache.js

@@ -93,7 +99,8 @@ export default class Cache {
      this.structs.filter(d => d.position.path === fsPath).pop(),
      this.variables.filter(d => d.position.path === fsPath).pop(),
      this.constants.filter(d => d.position.path === fsPath).pop(),
-      this.files.filter(d => d.position.path === fsPath).pop()
+      this.files.filter(d => d.position.path === fsPath).pop(),
+      this.cursor.filter(d => d.position.path === fsPath).pop()


Incorrect reference here also. cursor -> cursors

worksofliam · 2023-05-19T18:36:19Z

server/src/language/models/cache.js

@@ -79,6 +84,7 @@ export default class Cache {
      ...this.subroutines.map(def => def.name),
      ...this.variables.map(def => def.name),
      ...this.structs.map(def => def.name),
+      ...this.cursor.map(def => def.name),


cursor -> cursors.

worksofliam · 2023-05-19T18:36:30Z

server/src/language/models/cache.js

@@ -58,7 +62,8 @@ export default class Cache {
        files: [...this.files, ...second.files],
        structs: [...this.structs, ...second.structs],
        constants: [...this.constants, ...second.constants],
-        indicators: [...this.indicators, ...second.indicators]
+        indicators: [...this.indicators, ...second.indicators],
+        cursor: [...this.cursor, ...second.cursor]


cursor -> cursors

worksofliam · 2023-05-19T18:36:43Z

server/src/language/models/cache.js

@@ -136,7 +144,7 @@ export default class Cache {
  }

  clearReferences() {
-    [...this.parameters, ...this.constants, ...this.files, ...this.procedures, ...this.subroutines, ...this.variables, ...this.structs].forEach(def => {
+    [...this.parameters, ...this.constants, ...this.files, ...this.procedures, ...this.subroutines, ...this.variables, ...this.structs, ...this.cursor].forEach(def => {


cursor -> cursors

sebCIL · 2023-05-21T08:50:10Z

Sorry, I didn't test my test !

worksofliam · 2023-06-15T09:55:35Z

@sebCIL There is another big PR I am needing to merge ASAP. I suspect it will have a large impact on this PR. Just thought I'd give you a heads up.

sebCIL added 2 commits March 6, 2023 13:54

Show SQL cursors in outline view

4ecde9c

Add fixed format

6e59d06

worksofliam self-assigned this Mar 6, 2023

sebCIL changed the title ~~Show SQL cursors in outline view~~ [WIP] Show SQL cursors in outline view Mar 6, 2023

sebCIL added 2 commits March 7, 2023 13:40

fix fixed format

4f6ef7a

test cases

e1a28fd

sebCIL changed the title ~~[WIP] Show SQL cursors in outline view~~ Show SQL cursors in outline view Mar 8, 2023

worksofliam self-requested a review March 10, 2023 02:41

sebCIL mentioned this pull request Mar 10, 2023

Show SQL cursors in outline view #192

Open

Fix(cache)

f6402c3

Fix: Linter test

e019173

worksofliam requested changes May 15, 2023

View reviewed changes

sebCIL requested a review from worksofliam May 18, 2023 13:27

Fix: linter + SymbolKind

af0b9a0

worksofliam requested changes May 19, 2023

View reviewed changes

Fix: unit tests

6729ff0

sebCIL requested a review from worksofliam May 21, 2023 08:50

sebCIL added 2 commits October 17, 2023 13:55

Merge branch 'main' into feat_cursor

e699548

add type cursors

638ed80

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show SQL cursors in outline view #216

Show SQL cursors in outline view #216

sebCIL commented Mar 6, 2023 •

edited

Loading

sebCIL commented Mar 6, 2023

worksofliam commented Mar 6, 2023

sebCIL commented Mar 8, 2023

worksofliam commented Mar 19, 2023

sebCIL commented Mar 20, 2023

worksofliam commented May 12, 2023

chrjorgensen commented May 15, 2023

worksofliam left a comment

worksofliam May 15, 2023

worksofliam May 15, 2023

worksofliam May 15, 2023

worksofliam May 15, 2023

worksofliam May 15, 2023

worksofliam commented May 19, 2023

worksofliam left a comment

worksofliam May 19, 2023

worksofliam May 19, 2023

worksofliam May 19, 2023

worksofliam May 19, 2023

worksofliam May 19, 2023

sebCIL commented May 21, 2023

worksofliam commented Jun 15, 2023

Show SQL cursors in outline view #216

Are you sure you want to change the base?

Show SQL cursors in outline view #216

Conversation

sebCIL commented Mar 6, 2023 • edited Loading

Changes

Checklist

sebCIL commented Mar 6, 2023

worksofliam commented Mar 6, 2023

sebCIL commented Mar 8, 2023

worksofliam commented Mar 19, 2023

sebCIL commented Mar 20, 2023

worksofliam commented May 12, 2023

chrjorgensen commented May 15, 2023

worksofliam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

worksofliam commented May 19, 2023

worksofliam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebCIL commented May 21, 2023

worksofliam commented Jun 15, 2023

sebCIL commented Mar 6, 2023 •

edited

Loading