Add cli option to output all strings in plain text#688
Add cli option to output all strings in plain text#688Phuong-Le-NN wants to merge 1 commit intocoreos:mainfrom
Conversation
Adding --plain-text CLI flag to generate an Ignition config in plain text This generated config should be used for debugging only Fixes: coreos#670
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a --plain-text CLI flag for debugging, which generates an Ignition config with plain text strings instead of data URLs. The implementation correctly adds a new parameter to baseutil.MakeDataURL, a new option to TranslateOptions, and propagates the changes through various spec versions. The logic to skip JSON validation for plain text output is also correctly handled. I have two suggestions for improvement: one addresses an inconsistent use of the new flag in an experimental spec, and the other is a minor style fix to maintain code consistency.
| FilesDir string // allow embedding local files relative to this directory | ||
| NoResourceAutoCompression bool // skip automatic compression of inline/local resources | ||
| PlainTextEncoding bool // plain text, only for debugging | ||
| DebugPrintTranslations bool // report translations to stderr |
There was a problem hiding this comment.
This change introduces tabs for alignment within the TranslateOptions struct. The project's coding style appears to use spaces for alignment. To maintain consistency, please use spaces instead of tabs.
| FilesDir string // allow embedding local files relative to this directory | |
| NoResourceAutoCompression bool // skip automatic compression of inline/local resources | |
| PlainTextEncoding bool // plain text, only for debugging | |
| DebugPrintTranslations bool // report translations to stderr | |
| FilesDir string // allow embedding local files relative to this directory | |
| NoResourceAutoCompression bool // skip automatic compression of inline/local resources | |
| PlainTextEncoding bool // plain text, only for debugging | |
| DebugPrintTranslations bool // report translations to stderr |
There was a problem hiding this comment.
@Phuong-Le-NN this seems valid, just need to replace the tab by spaces.
|
|
||
| userCfgContent := []byte(buildGrubConfig(c.Grub)) | ||
| src, compression, err := baseutil.MakeDataURL(userCfgContent, nil, !options.NoResourceAutoCompression) | ||
| src, compression, err := baseutil.MakeDataURL(userCfgContent, nil, !options.NoResourceAutoCompression, false) |
There was a problem hiding this comment.
In this experimental spec, the plain-text option is not propagated to MakeDataURL for the user grub config. This is inconsistent with other experimental specs and the overall goal of the feature. To ensure consistent behavior for the --plain-text flag, options.PlainTextEncoding should be used here instead of hardcoding false.
| src, compression, err := baseutil.MakeDataURL(userCfgContent, nil, !options.NoResourceAutoCompression, false) | |
| src, compression, err := baseutil.MakeDataURL(userCfgContent, nil, !options.NoResourceAutoCompression, options.PlainTextEncoding) |
2 participants
Adding --plain-text CLI flag to generate an Ignition config in plain text This generated config should be used for debugging only
Fixes: #670