-
-
Notifications
You must be signed in to change notification settings - Fork 94
Add ability to import external snippets in bashly.yml #154
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
Conversation
|
@wolfgang42 this is your feature request - do you want to review? The split_config example shows how to use it: name: cli
help: Configuration splitting example
version: 0.1.0
commands:
# Import a command that is defined in another YAML file
- import: src/download_command.yml
# Import a command that is defined in the front matter of its own shell
# function.
- import: src/upload_command.sh |
| ==> hello colors | ||
| ===> hello colors | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and a couple of others were regenerated as a side effect, ignore.
|
|
||
| def config | ||
| @config ||= Config.new "#{Settings.source_dir}/bashly.yml" | ||
| @config ||= Config.new("#{Settings.source_dir}/bashly.yml").compose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where the composition happens, by just calling the #compose on the refined Hash, it will convert any import: path to the contents of the YAML represented by the path.
|
|
||
| def remove_front_matter | ||
| split(/^---\s*/).last | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is called in one place only, when loading user content.
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is the "heavy lifting". It adds refined functionality to hashes, and arrays that contain arrays or hashes, to allow import: yaml_path. It fails code climate's cognitive complexity test, but I figured its ok for now.
|
Docs added here |
|
Skimmed the docs briefly and it looks very promising. I'll try it out over the weekend, but I know you prefer a faster pace of development so don't let me stop you if you'd prefer to merge sooner. |
|
Excellent. Its a big enough feature, I would rather have your second opinion before merging. |
|
Just tried it out; it works the way I was hoping and I didn't run into any technical issues. The import system makes the bashly.yml file very easy to follow. However, I'm not sure about the design of the front matter. Bashly understands what's going on with YAML at the top of a shell script, but nothing else does, so it seems like there's great potential for problems here. Even for a human reader it's not obvious without knowing what's going on, and syntax highlighting definitely also gets in the way. (My editor picked up on an apostrophe in a YAML string and carried that through to the end of the file, making everything the wrong colors until I adjusted the quoting to make it happy.) I think it should probably be formatted as a comment, or maybe a specially marked HEREDOC or something, to keep it still as valid shell syntax—though I recognize that this will make it no longer plain YAML, and so will require special handling, I think that's the lesser of two evils in this case. |
Nothing else needs to.... except maybe a text editor.
You can't say this categorically. Front matter is a standard practice. It is the way to embed configuration in another document. You can't complain about syntax highlighting on one hand, and then propose it to be as a comment on the other... Personally - I think that the configuration should stay in the YAML, and the shall code in the shell script. But if someone insists on having them together, then I think a YAML front matter is the most natural and native way of doing it. I do not want to add any new inventions and hacks in the form of comments or specially marked anything. |
Well, text editors are a pretty big “something else.” But also the author might be using On the whole, I really think that the contents of a file named
I didn't mean to say it categorically, just that YAML is probably not the first thing most people would expect to see at the top of a file labeled as a shell script. Of course front matter is not uncommon, and I expect that most readers would figure out what they were looking at pretty quickly, but it is disconcerting.
Er, why not? My complaint about the syntax highlighting was that it's flat-out wrong, putting the YAML into a comment would fix that. (The YAML would still not be highlighted, of course, but at least the editor won't be trying any more.)
I can see your reasoning, and generally I think that following common standards like this would be a good thing. In this situation, though, I disagree that the front matter is most intuitive. To me it seems like more of a hack—perhaps not a hack in Bashly, but in all of the surrounding complications. Put it this way: though I'm the one that proposed this originally, I don't think I'll use it in it's current state; I think the downsides I've discussed above outweigh the convenience of having this built in. It seems to me like it needs more thought before it's ready to be added as a feature. |
Front matter existence contradicts this statement. It only ever exists in foreign files.
Why run tools on the partials, run them on the final.
Who is people? The only people looking at it are probably the same people who put it there.... I am currently leaning towards merging it, but I will wait for a few more days perhaps some improvement pops to mind. |
I thought you meant the fact that there is no syntax highlighting on the YAML, not that it scrambles the syntax highlighting of the shell script. The only case where I see it scrambling the shell syntax highlighting is when you have an unclosed quote, which is super easily fixable: hello: "User's folder"
---
echo "# this file is located in 'src/upload_command.sh'"
echo "# code for 'cli upload' goes here"
echo "# you can edit it freely and regenerate (it will not be overwritten)"
inspect_args |
Checklist
Key Changes
ComposeRefinementsmodule, which applies to hashes and arrays, and imports YAML files when it encounters a hash with"import" => "path"YAML::load_file.String#remove_front_matterextensionCloses #153