easyoptions icon indicating copy to clipboard operation
easyoptions copied to clipboard

Multi source files option

Open devingfx opened this issue 5 years ago • 4 comments

Hello

I needed easyoptions to parse documentation on bash sourced scripts for additional options. So I added a variable $easyoptions_include to the Bash implementation. (You can see it in the bash/example.sh file)

If you want to include this to your awesome work, I'm happy to share :)

devingfx avatar Apr 23 '19 18:04 devingfx

How about an array instead of a scalar, in case you want more than one extra file?

That said, this whole project needs an overhaul to use standard shell techniques such as here-docs, instead of jumping through complex hoops to find its own filename -- which cannot work reliably on all platforms.

On Wed., 24 Apr. 2019, 04:18 Thomas Di Grégorio, [email protected] wrote:

Hello

I needed easyoptions to parse documentation on bash sourced scripts for additional options. So I added a variable $easyoptions_include to the Bash implementation. (You can see it in the bash/example.sh file)

If you want to include this to your awesome work, I'm happy to share :)

You can view, comment on, or merge this pull request online at:

https://github.com/renatosilva/easyoptions/pull/15 Commit Summary

  • Add multi source files option in Bash version.
  • Update the Bash example with the multi source files option

File Changes

Patch Links:

  • https://github.com/renatosilva/easyoptions/pull/15.patch
  • https://github.com/renatosilva/easyoptions/pull/15.diff

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/renatosilva/easyoptions/pull/15, or mute the thread https://github.com/notifications/unsubscribe-auth/AABQFBK3J3KK46CLYVU67BTPR5HHJANCNFSM4HH5HFBA .

kurahaupo avatar Apr 24 '19 12:04 kurahaupo

Hi! I'm not sure to understand the difference in scalar/array in bash ... But it's already ok with my little modification to have several files listed separated with spaces:

easyoptions_include="file1 file2 file3"

or

easyoptions_include="$(ls dir | grep '.sh$')"

devingfx avatar Apr 26 '19 12:04 devingfx

Your contributed patch as it stands (with or without the recent suggestions) would not pass verification by ShellCheck; see ShellCheck.net.

This EasyOptions project is intended as a module that can be widely used in many projects. As such, it must reach a much higher standard of reliability than simply "works for me". It should anticipate weird corner cases and handle them gracefully. And it must not introduce security weaknesses. (That's the main reason I object to the methodology of re-reading the script to grep out the parameter definitions, when a heredoc would do more efficiently and more reliably. Unfortunately changing that would require adjustment to client scripts.)

Strongly adhering to ShellCheck is therefore important.

The first rule of ShellCheck is always double-quote variable expansions. If that stops you from using split-on-whitespace, that's a feature, not a bug. If you want more than one of something, use an array. Do not rely on splitting a string (scalar), as every possible separator token can also be part of a valid filename.

(The default "split on whitespace and then expand" behaviour is now considered a serious design flaw in the original shell. Modern shell scripts try hard to avoid expanding filenames after variable expansion.)

The assignment to an array should look like this:

easyoptions_include=( file1 file2 file3 )

easyoptions_include=( /some/dir/*.bash
                      ~/my/dir/*.sh
                      ~otheruser/their/dir/!(*~) )

At each point of use, change

$easyoptions_include

to

"${easyoptions_include[@]}"

All filename expansion (including globbing) is done only once, when the list is assigned to the array.

The '@' in the notation "${array[@]}" is magical; it overrules the "no splitting" that normally occurs with quotes, and makes a list of shell words, one for each element. (And yes, it makes zero words if there are zero elements.)

Note that the array assignment with globbing relies on nullglob to ensure that if there are no matches, the array will be empty; and the last expansion with !(*~) relies on globstar, to match all files except the ones ending in '~'. These features can be enabled by putting this at the top of the script:

shopt -s nullglob globstar

(In particular note that globstar slightly alters the way that scripts are parsed, so it must go at the top of the script before anything else is done. Note that this might impact some other parts of client the script, however in that case it too is probably failing ShellCheck.)

kurahaupo avatar Apr 26 '19 14:04 kurahaupo

Hi,

1 year and half later, and some more intensive usage of bash made me understand what you spoke about (bash arrays). I did the change in Bash implementation...

devingfx avatar Oct 20 '20 16:10 devingfx