Skip to content

feat: allow to pass FEEL language context via context #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Buckwich
Copy link
Member

@Buckwich Buckwich commented May 26, 2025

Related to camunda/camunda-modeler#4809

Proposed Changes

accept feelLanguageContext as an input for parserDialect and other feel specific fields

Test via:

npx @bpmn-io/sr bpmn-io/bpmn-js-properties-panel#4809-set-parser-dialect-camunda-context -l bpmn-io/properties-panel#4809-configurable-parser-dialect-context (doesn't work)

Manually link to bpmn-io/bpmn-js-properties-panel#4809-set-parser-dialect-camunda-context.

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label May 26, 2025
@Buckwich Buckwich changed the base branch from 4809-configurable-parser-dialect to main May 26, 2025 21:00
@Buckwich Buckwich force-pushed the 4809-configurable-parser-dialect-context branch from 03e762c to 2506a40 Compare May 26, 2025 21:05
@Buckwich Buckwich changed the title feat: allow to pass FEEL language context via provider feat: allow to pass FEEL language context via context May 27, 2025
@Buckwich Buckwich force-pushed the 4809-configurable-parser-dialect-context branch from 2506a40 to bfd7701 Compare May 27, 2025 10:02
@Buckwich Buckwich marked this pull request as ready for review May 27, 2025 10:45
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels May 27, 2025
@Buckwich Buckwich mentioned this pull request May 27, 2025
4 tasks
@Buckwich Buckwich force-pushed the 4809-configurable-parser-dialect-context branch from bfd7701 to f3785c0 Compare May 27, 2025 11:12
@Buckwich Buckwich requested review from nikku, a team and barmac and removed request for a team May 27, 2025 11:23
@Buckwich Buckwich requested a review from a team June 5, 2025 09:06
@Buckwich Buckwich marked this pull request as draft June 5, 2025 12:29
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Jun 5, 2025
@Buckwich
Copy link
Member Author

Buckwich commented Jun 5, 2025

  • camunda dialect is no longer applied to the feel popup after fixing merge conflicts, need to look into it again

@Buckwich Buckwich force-pushed the 4809-configurable-parser-dialect-context branch 4 times, most recently from e951b0a to 59b52fd Compare June 5, 2025 14:03
@Buckwich Buckwich marked this pull request as ready for review June 5, 2025 14:05
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jun 5, 2025
@nikku
Copy link
Member

nikku commented Jun 5, 2025

Thanks for addressing the merge conflicts. I'll look into this tomorrow. ✔️

@barmac
Copy link
Member

barmac commented Jun 6, 2025

I've added an @bpmn-io/sr script in the top comment for easy testing of the change.

@barmac
Copy link
Member

barmac commented Jun 6, 2025

Hmm the script fails due to missing context export, but it seems to be exported in the code.

@barmac
Copy link
Member

barmac commented Jun 6, 2025

It seems to work correctly when linked.
image

When linked to main on bpmn-js-properties-panel, the change does not break the editor (it's ignored and that's it).

Copy link
Member

@barmac barmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ from me on this change. Looking forward to Nico's feedback.

Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

#417 (comment) is what I'd consider to change.

@Buckwich Buckwich force-pushed the 4809-configurable-parser-dialect-context branch from 631a7ff to 0671478 Compare June 6, 2025 11:35
@nikku nikku force-pushed the 4809-configurable-parser-dialect-context branch from 0671478 to 454f0f3 Compare June 12, 2025 10:21
@nikku
Copy link
Member

nikku commented Jun 12, 2025

@Buckwich this looks like it is good to be merged 🙂

@Buckwich
Copy link
Member Author

I didn't want to interfere with the other fixes/issues in properties panel for the current release. I'll wait to merge until those are properly integrated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Review pending
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants