Vml Library Support #1602

joswig · 2025-01-23T01:32:53Z

resolves #1610

goetzrrGit

I will approve this as I’ll be out tomorrow. If you’d like to merge, please feel free to do so.

We’ve already discussed some issues:

Autocomplete doesn’t pop up with Control+Space; please verify with Bryan (non-blocking).
SeqN’s Selected command panel needs full hookups for library sequences like VML (see [GitHub issue #1550]). (non-blocking)

No big issues that I ran into.

src/components/sequencing/SequenceEditor.svelte

src/utilities/codemirror/vml/vmlTooltip.ts

goetzrrGit · 2025-02-05T22:01:36Z

src/utilities/codemirror/vml/vmlTreeUtils.ts

+    if (statementNode) {
+      const vmManagementNode = statementNode.getChild(RULE_VM_MANAGEMENT);
+      if (vmManagementNode) {
+        const spawnNode = vmManagementNode.getChild(RULE_SPAWN);
+        if (spawnNode) {
+          return spawnNode.getChild(RULE_CALL_PARAMETERS);
+        }
+      }
+
+      // ISSUE and ISSUE_DYNAMIC
+      return statementNode.firstChild?.getChild(RULE_CALL_PARAMETERS) ?? null;
+    }
+    return null;


optional: Always found that make conditionals top level and removing nesting more readability.

if (!statementNode) { return null; } const vmManagementNode = statementNode.getChild(RULE_VM_MANAGEMENT); if (!vmManagementNode) { return statementNode.firstChild?.getChild(RULE_CALL_PARAMETERS) ?? null; } const spawnNode = vmManagementNode.getChild(RULE_SPAWN); if (!spawnNode) { return null; } return spawnNode.getChild(RULE_CALL_PARAMETERS);

src/utilities/codemirror/vml/vmlTreeUtils.ts

src/components/sequencing/CommandPanel/SelectedCommand.svelte

duranb · 2025-02-06T02:19:52Z

src/components/sequencing/SequenceEditor.svelte

-          workspace_id: sequence.workspace_id,
-        };
-      });
+    if (isInVmlMode) {


Can we pull this logic out into a util function so we can add some unit tests to it?

duranb · 2025-02-06T02:22:56Z

src/components/sequencing/SequenceEditor.svelte

+            name: sequence.name,
+            parameters: parseVariables(tree.topNode, sequence.definition, 'ParameterDeclaration') ?? [],
+            tree,
+            type: 'librarySequence',


Can we make librarySequence an enum?

sure, or remove it depending on your take on #1602 (comment)

duranb · 2025-02-06T02:28:55Z

src/components/sequencing/form/ByteArrayEditor.svelte

+    }
+  }
+</script>
+


Can this be wrapped in a <div></div> so that it doesn't affect the parent component's hierarchy in an unexpected way?

duranb · 2025-02-06T02:29:40Z

src/types/sequencing.ts

@@ -155,9 +155,16 @@ export type LibrarySequence = {
  name: string;
  parameters: VariableDeclaration[];
  tree: Tree;
+  type: 'librarySequence';


Are there other type strings that this can be?

no, I added it to keep the associated type guard trivial and match the pattern we're using with FswCommand.

Do you have a preference on avoiding explicit typing?

Ohh I see it now. I don't necessarily have an issue with the explicit typing, but it's not intuitive as to what the purpose was. There's nothing relating the LibrarySequence type to a FswCommand type unless you have enough context of how it's being used and that might cause some issues in the future if we have to change the Command types. I think ideally, these types can extend from a common interface. If that's too out of this scope, then I think making the type an enum that's shared with all the command types would suffice for now.

src/utilities/codemirror/vml/vmlAdaptation.ts

duranb · 2025-02-06T02:40:47Z

src/utilities/codemirror/vml/vmlAdaptation.ts

+      const cursorLine = context.state.doc.lineAt(selection.ranges[0].to);
+      const cursorLineTrimmed = cursorLine.text.trim();
+
+      if (!cursorLineTrimmed) {


Can we change this to compare cursorLineTrimmed explicitly with an "" to make it more apparent what the comment on the following line means?

Addressed in b848d63

duranb · 2025-02-06T02:44:57Z

src/utilities/codemirror/vml/vmlBlockLibrary.ts

+    case 'UINT':
+    case 'FLOAT':
+      return {
+        arg_type: { FLOAT: 'float', INT: 'integer', UINT: 'unsigned' }[variable.type] as


This seems a little forced. I think I'd be okay with this being a little repetitive for the sake of being more clear if each number case returned almost the same object.

Addressed in 284aa51

duranb · 2025-02-06T02:48:25Z

src/utilities/codemirror/vml/vmlLinter.ts

 function validateArguments(
  commandDictionary: CommandDictionary,
  commandDef: FswCommand,
  functionNode: SyntaxNode,
  functionNameNode: SyntaxNode,
  docText: string,
+  // issue_dynamic puts the stem into the first argument


Do you mind fleshing this comment out a little bit more to be clearer to someone who's not too familiar with VML?

Addressed in fa3ed42

duranb · 2025-02-06T02:50:39Z

src/utilities/codemirror/vml/vmlTooltip.ts

+    above: true,
+    create() {
+      const dom = document.createElement('div');
+      try {


Is something generating an error here for a try/catch to be necessary? It's not immediately clear as to what that would be. StringTooltip.svelte isn't throwing anything.

This was to satisfy SonarQube, I'll look for another way

Oh weird. It's not flagging the other tooltips though, right?

I believe they haven't been scanned yet since they are unchanged in the PR

sonarqubecloud · 2025-02-07T01:37:07Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

joswig temporarily deployed to test-workflow January 23, 2025 01:33 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow January 23, 2025 06:01 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow January 29, 2025 01:05 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow January 29, 2025 01:06 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow January 29, 2025 21:00 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow January 30, 2025 22:09 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow January 30, 2025 23:40 — with GitHub Actions Inactive

joswig had a problem deploying to test-workflow January 31, 2025 19:10 — with GitHub Actions Failure

joswig temporarily deployed to test-workflow February 1, 2025 12:04 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow February 2, 2025 04:05 — with GitHub Actions Inactive

joswig force-pushed the feat/vml_libraries branch from 04fbbe2 to 1acdc23 Compare February 2, 2025 04:05

joswig temporarily deployed to test-workflow February 2, 2025 04:05 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow February 2, 2025 04:18 — with GitHub Actions Inactive

joswig had a problem deploying to test-workflow February 2, 2025 16:25 — with GitHub Actions Failure

joswig temporarily deployed to test-workflow February 2, 2025 22:24 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow February 3, 2025 17:25 — with GitHub Actions Inactive

joswig force-pushed the feat/vml_libraries branch from 6bded13 to 553ee27 Compare February 3, 2025 17:31

joswig temporarily deployed to test-workflow February 3, 2025 17:31 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow February 3, 2025 22:18 — with GitHub Actions Inactive

joswig force-pushed the feat/vml_libraries branch from 0f3fcfe to 2be0db9 Compare February 3, 2025 23:52

joswig temporarily deployed to test-workflow February 3, 2025 23:52 — with GitHub Actions Inactive

joswig temporarily deployed to test-workflow February 4, 2025 21:36 — with GitHub Actions Inactive

joswig added 8 commits February 4, 2025 13:56

grammar updates to allow date ranges and validate issue_dynamic

444918d

checks that spawned block names exist

91dcdd5

wired up issue_dynamic form editor

9255fb5

snippets for some block structures

68dde78

validate issue_dynamic arguments

20fdf4f

support SPACECRAFT_TIME() command and encoded_string arg

b44a719

tooltip for encoded string

de2ceaa

st warning

be2d2d4

joswig temporarily deployed to test-workflow February 5, 2025 19:19 — with GitHub Actions Inactive

exempt variable declarations from checking if in scope

44c22e6

joswig temporarily deployed to test-workflow February 5, 2025 20:36 — with GitHub Actions Inactive

goetzrrGit approved these changes Feb 6, 2025

View reviewed changes

duranb requested changes Feb 6, 2025

View reviewed changes

spelling

56da517

joswig temporarily deployed to test-workflow February 6, 2025 17:24 — with GitHub Actions Inactive

expand docs on issue vs issue_dynamic

fa3ed42

joswig had a problem deploying to test-workflow February 6, 2025 17:38 — with GitHub Actions Failure

split up numerci cases

284aa51

joswig temporarily deployed to test-workflow February 6, 2025 17:42 — with GitHub Actions Inactive

readability of content assist

b848d63

joswig had a problem deploying to test-workflow February 6, 2025 23:29 — with GitHub Actions Failure

stricter types

43347ef

joswig temporarily deployed to test-workflow February 6, 2025 23:34 — with GitHub Actions Inactive

readability of completion code

5a03821

joswig temporarily deployed to test-workflow February 7, 2025 01:03 — with GitHub Actions Inactive

add comment to make SeqN adaptation symmetric

7a223db

joswig temporarily deployed to test-workflow February 7, 2025 01:07 — with GitHub Actions Inactive

reduce conditional nesting

8dc4e9f

joswig temporarily deployed to test-workflow February 7, 2025 01:12 — with GitHub Actions Inactive

fix svelte-check lint

e91003d

joswig temporarily deployed to test-workflow February 7, 2025 01:14 — with GitHub Actions Inactive

remove extraneous truthyness check

bef1849

joswig temporarily deployed to test-workflow February 7, 2025 01:23 — with GitHub Actions Inactive

reduce nesting

dc4c0d9

joswig temporarily deployed to test-workflow February 7, 2025 01:30 — with GitHub Actions Inactive

reduce nesting

0442642

joswig temporarily deployed to test-workflow February 7, 2025 01:36 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vml Library Support #1602

Vml Library Support #1602

joswig commented Jan 23, 2025 •

edited

Loading

goetzrrGit left a comment

goetzrrGit Feb 5, 2025

duranb Feb 6, 2025

duranb Feb 6, 2025

joswig Feb 6, 2025

duranb Feb 6, 2025

duranb Feb 6, 2025

joswig Feb 6, 2025

duranb Feb 6, 2025

duranb Feb 6, 2025

joswig Feb 6, 2025

duranb Feb 6, 2025

joswig Feb 6, 2025

duranb Feb 6, 2025

joswig Feb 6, 2025

duranb Feb 6, 2025

joswig Feb 6, 2025

duranb Feb 6, 2025

joswig Feb 6, 2025

sonarqubecloud bot commented Feb 7, 2025

Vml Library Support #1602

Are you sure you want to change the base?

Vml Library Support #1602

Conversation

joswig commented Jan 23, 2025 • edited Loading

goetzrrGit 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

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

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

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 7, 2025

Quality Gate passed

joswig commented Jan 23, 2025 •

edited

Loading