Skip to content

[Feat] Determine the format automatically for input files on the CLI #2267

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

Merged
merged 6 commits into from
Jun 2, 2025

Conversation

yannham
Copy link
Member

@yannham yannham commented May 23, 2025

Depends on #2266

Supporting multiple input formats on the CLI

Motivation

When importing a file, Nickel determines the format automatically by looking at the file's extension. This is not true on the CLI though, which could be useful: for example, one could use Nickel as a merging engine for JSON without actually needing to migrate anything to Nickel: nickel base.json ext.json foo.json. In conjunction with #2266, supporting other formats on the CLI could also allow validating non-Nickel config in a lightweight and non-invasive way: nickel eval config.json --apply-contract validation/*.ncl.

Content

Instead of always determining the input format to be Nickel for files coming from the CLI, this PR uses the same logic as for imports, looking at the file's extension.

Shortcomings

One current shortcoming is that, as opposed to imports, there is no way to override the format selection logic. However, we've lived with that for a long time with imports - the explicit format syntax is relatively recent -, and the motivation for imports is a bit different. The explicit format syntax for imports is more about specifying the format explicitly when Nickel doesn't recognize the extension, say with foo.data that should be text, rather than overriding an import named foo.json to be understood as TOML instead. The latter would be a very strange use-case.

Thus, this PR should be a strict improvement over the previous situation, which would consider foo.json as a Nickel input, and shouldn't require any work-around, although it could be extended in some way to specify the format explicitly.

@yannham
Copy link
Member Author

yannham commented May 23, 2025

Hmm, the snapshot test failure happens on the CI but not on my machine...

@yannham yannham added this to the Next minor (1.12) milestone May 23, 2025
@yannham yannham requested a review from jneem May 25, 2025 21:46
Base automatically changed from feat/cli-contract-arg to master May 25, 2025 21:53
Copy link

dpulls bot commented May 25, 2025

🎉 All dependencies have been resolved !

@yannham yannham force-pushed the feat/multi-format-cli branch from 0b9a1d0 to 70a12d5 Compare May 26, 2025 09:24
@yannham yannham force-pushed the feat/multi-format-cli branch from 70a12d5 to 571237a Compare May 26, 2025 09:26
@yannham yannham requested a review from nbacquey May 27, 2025 11:44
Copy link
Member

@nbacquey nbacquey left a comment

Choose a reason for hiding this comment

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

Otherwise lgtm

@yannham yannham force-pushed the feat/multi-format-cli branch from 57b3464 to 43e41a9 Compare May 27, 2025 15:54
@yannham yannham enabled auto-merge May 27, 2025 15:54
@yannham yannham force-pushed the feat/multi-format-cli branch from 86cf728 to 555a32c Compare May 28, 2025 15:18
Copy link
Contributor

github-actions bot commented May 28, 2025

@yannham yannham force-pushed the feat/multi-format-cli branch from 0095d96 to 896b23a Compare June 2, 2025 10:44
@yannham yannham added this pull request to the merge queue Jun 2, 2025
Merged via the queue into master with commit c234d2d Jun 2, 2025
5 checks passed
@yannham yannham deleted the feat/multi-format-cli branch June 2, 2025 11:04
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.

3 participants