Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

#288 Remove space between arg name and value, support of arg repeating with each value#310

Merged
bugy merged 3 commits intobugy:masterbugy/script-server:masterfrom
miksir:space_after_arg_and_multiple_argmiksir/script-server:space_after_arg_and_multiple_argCopy head branch name to clipboard
Jun 11, 2020
Merged

#288 Remove space between arg name and value, support of arg repeating with each value#310
bugy merged 3 commits intobugy:masterbugy/script-server:masterfrom
miksir:space_after_arg_and_multiple_argmiksir/script-server:space_after_arg_and_multiple_argCopy head branch name to clipboard

Conversation

@miksir
Copy link

@miksir miksir commented Jun 10, 2020

Issue #288

@miksir miksir changed the title [#288] Remove space between arg name and value, support of arg repeating with each value #288 Remove space between arg name and value, support of arg repeating with each value Jun 10, 2020
@bugy
Copy link
Owner

bugy commented Jun 10, 2020

Hi @miksir , thanks for the PR, the changes look good to me, but I have some concerns regarding naming.
I don't like no_space, to be honest, because actually it's not about space, but about having param and value passed as a single argument (e.g. if in a value you have spaces, it will still be only one argument)

Regarding repeat_arg: at the moment for the -p (or --flag, or ---arg=) i use config called param. I think for consistency it would be better to keep it.
OR: i googled for proper naming: https://stackoverflow.com/questions/36495669/difference-between-terms-option-argument-and-parameter So in this case param was a bad name from me. The proper one would be "option".
So i think we should either use repeat param or repeat option.

What do you think?

@miksir
Copy link
Author

miksir commented Jun 10, 2020

I am really dont care about naming :) Just need time for search and replace :) Will try to rename tomorrow.

@miksir
Copy link
Author

miksir commented Jun 10, 2020

So any suggestions how to call param_space? :) param_value_split? :)

repeat_arg to repeat_param? or repeat_param_with_each_value?

Renaming param to option require renaming of param field, and it's too complicated

@bugy
Copy link
Owner

bugy commented Jun 10, 2020

repeat_param would be ok, as for me
and may be single_arg_param ?

@bugy
Copy link
Owner

bugy commented Jun 11, 2020

Or may be be same_arg_param

MiksIr added 2 commits June 11, 2020 12:52
+ Option for add arg name with each value of multioption type
@miksir miksir force-pushed the space_after_arg_and_multiple_arg branch from d5ddb9b to 280a5ac Compare June 11, 2020 10:03
@miksir
Copy link
Author

miksir commented Jun 11, 2020

Renamed

Copy link
Owner

@bugy bugy left a comment

Choose a reason for hiding this comment

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

Great job, thank you!

@bugy
Copy link
Owner

bugy commented Jun 11, 2020

There are 2 web tests failing, would you mind having a look? Or should I?

@miksir
Copy link
Author

miksir commented Jun 11, 2020

Oh, I will check.

@miksir
Copy link
Author

miksir commented Jun 11, 2020

Probably fired because I changed visibility of some fields when they are unused (they was disabled before)

@bugy bugy merged commit b164c8b into bugy:master Jun 11, 2020
@bugy bugy added this to the 1.16.0 milestone Jun 11, 2020
@miksir miksir deleted the space_after_arg_and_multiple_arg branch June 12, 2020 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.