Import SATF Library Sequence #1592

goetzrrGit · 2025-01-14T21:49:12Z

Closes #1591

Library Sequence Modal:

Introduced a new modal for importing a library sequence file.
Integrated the library modal with workspaces for seamless interaction.

Linting Bugfix:

Fixed an issue where the linter incorrectly reported errors for values within a valid range when multiple ranges were defined.
Fixed an issue where the linter incorrectly reported errors for command stems with no arguments.

Autocomplete Bugfix:

Improved autocomplete functionality to allow completion even with invalid variable types. The user will be notified of the invalid type within the editor and will need to manually correct it.

SATF/SASF Library

Fixed an issue where we were not generating the @model correctly.

TESTING

Download the attach file (banana_library.txt)
Rename it to banana_library.satf
Create a new workspace
In the middle panel underneath the 'create sequence' you should see a 'import library'
Click on 'import library' and select a parcel and library file ex. banana_library.satf
Verify the library gets imported and you see 3 sequences created

banana_library.txt

cohansen · 2025-01-14T22:41:18Z

Is this branch supposed to merge into #1574? It looks like a lot of the same changes.

goetzrrGit · 2025-01-15T14:54:17Z

Is this branch supposed to merge into #1574? It looks like a lot of the same changes.

Yeah it is branched from 1574 and since that was merged in the first 4 fell off. It should now all be new code

duranb · 2025-01-17T19:51:31Z

src/components/sequencing/Sequences.svelte

@@ -86,6 +125,18 @@
        >
          New Sequence
        </button>
+
+        <button


Can we change the flow of this container to keep these buttons on the same line instead of wrapping to the next line if there isn't enough screen space?

duranb · 2025-01-17T19:53:54Z

src/components/sequencing/Sequences.svelte

@@ -59,6 +63,41 @@
    const workspaceId = getSearchParameterNumber(SearchParameters.WORKSPACE_ID);
    goto(`${base}/sequencing/new${workspaceId ? `?${SearchParameters.WORKSPACE_ID}=${workspaceId}` : ''}`);
  }
+
+  async function importLibarySequences(): Promise<void> {
+    const workspaceId = getSearchParameterNumber(SearchParameters.WORKSPACE_ID);


workspaceId already exists in the component as a state variable. Can we use that instead of fetching it from the URL again?

Thanks for pointing that out

duranb · 2025-01-17T20:09:39Z

src/components/sequencing/Sequences.svelte

+  async function importLibarySequences(): Promise<void> {
+    const workspaceId = getSearchParameterNumber(SearchParameters.WORKSPACE_ID);
+    if (workspaceId === null) {
+      console.log("Workspace doesn't exist");


Our current convention for most complicated modals is to call them from an effects function as well as showing toasts. Would you mind moving this into one?

Ahhh, got it. I moved part of this function to the effects.ts

duranb · 2025-01-17T20:12:37Z

src/components/sequencing/Sequences.svelte

+    if (confirm && value) {
+      const fileName = await value.libraryFile.name;
+      const type = fileName.slice(fileName.lastIndexOf('.') + 1);
+      if (type !== 'satf') {


Are files that contain satf code, but don't have the extension on it allowed? If the file extension is required, then you can specify valid file extensions to disallow unsupported files from even being selected also.

Good point. I add accept=".satf" to only allow for satfs to be imported. From what I have seen these files are always appended with .satf

duranb · 2025-01-17T20:21:14Z

src/components/sequencing/Sequences.svelte

+        showFailureToast('Library Import: Unsupported file type');
+        return;
+      }
+      const contents = await value.libraryFile.text();


I think you might need to add a try/catch for if it's unable to read the file.

Got it I added a try-catch

duranb · 2025-01-17T20:27:47Z

src/utilities/codemirror/codemirror-utils.ts

            : 'UNKNOWN';
      default:
-        throw Error(`unknown argument type ${parameter.type}`);
+        console.log(`Unknown argument type ${parameter.type}`);


I think the util file should not be in charge of logging. It would be more reusable if the consuming functions using this function are responsible for how they report errors.

Got it. I removed the logging. It is returning a value with an error so it stand out to the users that the variable type is incorrect.

The issues is the auto-complete feature for variables will attempts to fill in values for the user by default. These values are coming from a parent sequence file being invoked or called by the @ACTIVATE or @LOAD directive . If there is an issue I still want to display valid parameters.

ex.
C @ACTIVATE("SEQ1") 1 "hello" ERROR:tempurture FALSE

Before the autocomplete will fail and the user will not see any arguments. In order to see what these argument are the user will have to open "SEQ1" in a new tab. Also it lets them know that SEQ1 has an issue so go fix it.

duranb · 2025-01-17T20:29:12Z

src/components/sequencing/Sequences.svelte

+      const fileName = await value.libraryFile.name;
+      const type = fileName.slice(fileName.lastIndexOf('.') + 1);
+      if (type !== 'satf') {
+        console.log(`Unsupported file type ${type}`);


Do we need to log here if we're already showing the toast?

Nope, just for debugging. I removed it.

This modal allows you to select a parcel and import a library sequence file

The linter incorrectly reported errors for values within a valid range when multiple ranges were defined. For example, if the ranges were 5-10 and 11-20, the linter would erroneously flag the value 8 as invalid because it falls outside the 11-20 range, even though it is valid within the 5-10 range.

…invalid variable types. Allow autocomplete to succeed and show the ERROR parameter value. The user will see this error in the editor and manually have to fix it.

The linter will complain if there is a command with no arguments even though the command itself doesn't have any argument defined in the command dictionary.

There shouldn't be any parenthesis and commas in the SeqN generation

goetzrrGit added feature New feature or request sequencing Anything related to the sequencing domain labels Jan 14, 2025

goetzrrGit requested review from cohansen, duranb and joswig January 14, 2025 21:49

goetzrrGit self-assigned this Jan 14, 2025

goetzrrGit requested a review from a team as a code owner January 14, 2025 21:49

goetzrrGit requested a review from AaronPlave January 14, 2025 21:49

goetzrrGit had a problem deploying to test-workflow January 14, 2025 21:49 — with GitHub Actions Failure

goetzrrGit had a problem deploying to test-workflow January 14, 2025 22:04 — with GitHub Actions Failure

goetzrrGit had a problem deploying to test-workflow January 14, 2025 22:05 — with GitHub Actions Failure

Bugfix allow for paths in Headers

405ca45

goetzrrGit force-pushed the import_satf_library branch from 4c910c8 to 98330e2 Compare January 15, 2025 14:52

goetzrrGit had a problem deploying to test-workflow January 15, 2025 14:52 — with GitHub Actions Failure

goetzrrGit had a problem deploying to test-workflow January 15, 2025 14:57 — with GitHub Actions Failure

goetzrrGit had a problem deploying to test-workflow January 15, 2025 14:58 — with GitHub Actions Failure

duranb requested changes Jan 17, 2025

View reviewed changes

goetzrrGit added 6 commits January 22, 2025 06:53

Library Sequence Modal

44e661b

This modal allows you to select a parcel and import a library sequence file

Hook up the library modal to workspaces

2eb0ba4

Improved autocomplete functionality by allowing completion even with …

9cd7a66

…invalid variable types. Allow autocomplete to succeed and show the ERROR parameter value. The user will see this error in the editor and manually have to fix it.

Fixed linter error for commands without arguments.

1914a6f

The linter will complain if there is a command with no arguments even though the command itself doesn't have any argument defined in the command dictionary.

Fix @model generation.

f45cb60

There shouldn't be any parenthesis and commas in the SeqN generation

goetzrrGit force-pushed the import_satf_library branch from 98330e2 to f45cb60 Compare January 22, 2025 16:58

goetzrrGit temporarily deployed to test-workflow January 22, 2025 16:58 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import SATF Library Sequence #1592

Import SATF Library Sequence #1592

goetzrrGit commented Jan 14, 2025 •

edited

Loading

cohansen commented Jan 14, 2025

goetzrrGit commented Jan 15, 2025

duranb Jan 17, 2025

duranb Jan 17, 2025

goetzrrGit Jan 22, 2025

duranb Jan 17, 2025

goetzrrGit Jan 22, 2025

duranb Jan 17, 2025

goetzrrGit Jan 22, 2025 •

edited

Loading

duranb Jan 17, 2025

goetzrrGit Jan 22, 2025

duranb Jan 17, 2025

goetzrrGit Jan 22, 2025 •

edited

Loading

duranb Jan 17, 2025

goetzrrGit Jan 22, 2025

Import SATF Library Sequence #1592

Are you sure you want to change the base?

Import SATF Library Sequence #1592

Conversation

goetzrrGit commented Jan 14, 2025 • edited Loading

cohansen commented Jan 14, 2025

goetzrrGit commented Jan 15, 2025

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

goetzrrGit Jan 22, 2025 • edited Loading

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

goetzrrGit Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goetzrrGit commented Jan 14, 2025 •

edited

Loading

goetzrrGit Jan 22, 2025 •

edited

Loading

goetzrrGit Jan 22, 2025 •

edited

Loading