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

Feat additionnal project yaml #128

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JuPrgn
Copy link

@JuPrgn JuPrgn commented Dec 21, 2023

Fix #117

Add a feature to use Ceedling project: argument in order to allow to merge/load custom configuration from an additional project_specific.yml to the main project.yml.

This is usefull to test on different compiler or for different hardware as you can keep you common settings on main project.yml and use multiple specific.yml files to changes flags, defines... at runtime.

@numaru
Copy link
Owner

numaru commented Jan 2, 2024

The data in project.yml is also read by the extension and any additional .yml file merged at runtime should be considered the same.

  • The translation from the option names to .yml files should be done using the [:project][:options_paths] setting in project.yml
  • The option files should be read and merged in getYmlProjectData()
  • The option files should be watched for change by watchFilesForReload()

@@ -337,7 +342,9 @@ export class CeedlingAdapter implements TestAdapter {
}

private getCeedlingCommand(args: ReadonlyArray<string>) {
const line = `ceedling ${args.join(" ")}`;
const projectCustom = this.getProjectCustom();
const project = projectCustom ? `project:${projectCustom} ` : '';
Copy link
Owner

Choose a reason for hiding this comment

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

The documentation define a options: parameter. Is project: an alias?

Copy link
Author

Choose a reason for hiding this comment

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

I have always used project: probably this was documented this way in previous versions of Ceedling but I will do some tests with options: and update this PR

@@ -77,6 +77,12 @@
"default": ".",
"scope": "resource"
},
"ceedlingExplorer.projectCustom": {
Copy link
Owner

Choose a reason for hiding this comment

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

I think it could be named options to keep the same ceedling vocabulary. It should also be a list of name.

@numaru
Copy link
Owner

numaru commented Jan 2, 2024

Thank you for the contribution. I prefer to not include the PR in today's release because I think we can improve the support for options files and I don't want to change the ceedlingExplorer settings too often.

@JuPrgn
Copy link
Author

JuPrgn commented Jan 2, 2024

Thank you for your feedback I will improve this PR soon following your recommendations.
If I understand it well you propose to merge optional data to a single project.yml that will be used by getYmlProjectData() ? Merging project.yml and optional.yml datas could be an issue if some key are defined twice ? this is a check that is performed by the extension right ?

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.

does VSCode extension support "options:<custom_yaml_file>" ?
2 participants