Skip to content
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

--json and --json-file CLI args add to lfc #1686

Merged
merged 6 commits into from
Apr 7, 2023
Merged

--json and --json-file CLI args add to lfc #1686

merged 6 commits into from
Apr 7, 2023

Conversation

patilatharva
Copy link
Collaborator

Objective

Allow properties to be passed into lfc through a JSON string or file as an alternative to CLI arguments.

Reason

To provide a JSON interface between lingo and lfc.

Tasks

  • Make options {input files, --json, --json-files} mutually exclusive.
  • Refactor run() (entry point of CliBase.java) to first check for --json and --json-file args, and if either are given, unpack args and recurse into run().
  • Implement method jsonStringToArgs to convert a JSON string to a Lingua Franca args array.
  • Add error handling for file reads and JSON indexing.
  • Add tests for --json and --json-file.

@patilatharva patilatharva linked an issue Apr 4, 2023 that may be closed by this pull request
@patilatharva patilatharva self-assigned this Apr 4, 2023
@tanneberger
Copy link
Member

I am tested it and it works see this pull request

@lhstrh lhstrh marked this pull request as ready for review April 4, 2023 15:29
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

This looks great! Only nitpicks from my side...

org.lflang/src/org/lflang/cli/CliBase.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/cli/CliBase.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/cli/CliBase.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/cli/Lff.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/cli/Lfc.java Outdated Show resolved Hide resolved
lhstrh and others added 2 commits April 4, 2023 22:24
The error gets reported with an unexpected line number (not even in the
right file) when a mistake is inserted into the type for the list of
time values. I do not want to fix this right now (honestly it is the C++
compiler's fault, I do not even know how we would fix this), so instead
I will try to cover it up.
@patilatharva
Copy link
Collaborator Author

Thanks for the suggestions and fixes @lhstrh and @petervdonovan - do you think the PR is in good shape for merging now?

Copy link
Collaborator

@petervdonovan petervdonovan left a comment

Choose a reason for hiding this comment

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

I think this is ready to merge. My only comments are unimportant.

org.lflang.tests/src/org/lflang/tests/cli/LfcCliTest.java Outdated Show resolved Hide resolved
org.lflang.tests/src/org/lflang/tests/cli/LfcCliTest.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/cli/CliBase.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/cli/CliBase.java Show resolved Hide resolved
lhstrh and others added 2 commits April 7, 2023 09:17
@lhstrh lhstrh merged commit 422d06f into master Apr 7, 2023
@oowekyala oowekyala mentioned this pull request May 2, 2023
@cmnrd cmnrd deleted the cli-json-option branch June 8, 2023 07:45
@tanneberger tanneberger mentioned this pull request Aug 22, 2023
4 tasks
@petervdonovan petervdonovan added the feature New feature label Aug 25, 2023
@lhstrh lhstrh changed the title Add --json and --json-file CLI args to Lfc --json and --json-file CLI args add to lfc Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add --json CLI arg to Lfc
4 participants