Skip to content

Conversation

@ChristopherHX
Copy link
Contributor

✍ Changes

Proof of concept, feel free to change this as needed

Now both get parsed and added to settings pane

    - run: ${{ secrets.OPEN_VSX_TOKEN3 && 'ok' }}
    - run: ${{ secrets['OPEN_VSX_TOKEN4'] && 'ok' }}

Inside the index visitor, I didn't liked the visitor pattern anymore and used instanceOf instead of creating another visitor

📋 Checklist

  • I tested my changes, not really, but I did try this change locally
  • I updated relevant documentation
  • I added myself to the contributors' list

Closes #163

@ChristopherHX ChristopherHX force-pushed the use-actions-workflow-parser-for-auto-settings branch from 1b0fbc6 to 3ccaf89 Compare February 11, 2025 19:19
@ChristopherHX
Copy link
Contributor Author

Note seems like every expression with errors are skipped without showing errors, so in that point of view no new behavior.
Now this works as well

if: vars['OPEN_VSX_TOKEN8']

@ChristopherHX
Copy link
Contributor Author

Possible should get a deep performance analysis due to #174

The problem in this impl is that each workflow is parsed more than once

@SanjulaGanepola
Copy link
Owner

@ChristopherHX Thanks for taking a look at this. Could you resolve the merge conflict as a couple PRs I had got merged in.

I will review this PR in the coming days. By chance, did you follow some reference implementation/documentation? If yes, can you point me to that code as well? It will help me better understand this implementation.

Possible should get a deep performance analysis due to #174

The main performance problem here is that the extension is constantly writing and reading from the extensions globalState especially when settings change which is not good. I am going to take a look at implementing the suggestion in #173 to address this.

@ChristopherHX
Copy link
Contributor Author

The main performance problem here is that the extension is constantly writing and reading from the extensions globalState especially when settings change which is not good. I am going to take a look at implementing the suggestion in #173 to address this.

I observed as soon you enable / disable a secret/setting the whole treeview reads all workflow files gain, this might also not be nothing. (for the manual refresh this is ok)

@ChristopherHX
Copy link
Contributor Author

By chance, did you follow some reference implementation/documentation?

No, I have written this from scratch and eventually looked into the source code as well. I knew there must be a traverse function of the raw parsing result from c#, while the once I use is from 2019 it looks pretty similar with the 2023 js version.

I'm working with the c# codebase of these packages since a long time e.g. from the c# side I already had written such a function, which seems to have far less lines of code
https://github.com/ChristopherHX/runner.server/blame/3303770cf14660779fb3012015a861531d587248/src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateTokenExtensions.cs#L216-L238

@SanjulaGanepola
Copy link
Owner

@ChristopherHX FYI, due to the performance issues with how settings are stored, I am planning to rework that before having a look at this as those changes might impact what is done here. I have been a bit busy lately, but will try to get something soon.

@ChristopherHX
Copy link
Contributor Author

No problem, I'm busy as well with my other projects.

My primary goal was to show you, what could be done by the official Actions npm packages.

I know, I'm writing 2 weeks later xd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate if @actions/expressions and @actions/workflow-parser can be used as parsers instead of regular expressions

2 participants