-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Avoid add M exec-opt twice #412
Conversation
If user explicitly specify `-M:xxx` option to e.g., add some aliases, we should just take it instead of build second `-M` in MAIN_FLAG.
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.
@bootleq Thanks for your contribution!
Looks good to me, but just sorry, could you fix the following file instead of bin/iced
?
vim-iced/clj/template/iced.bash
Lines 404 to 408 in ed226c7
if [ "$(printf '%s\n' "$MIN_MAIN_VERSION" "$CLOJURE_CLI_VERSION" | sort -V | head -n1)" = "$MIN_MAIN_VERSION" ]; then | |
MAIN_FLAG="-M -m" | |
else | |
MAIN_FLAG="-m" | |
fi |
bin/iced
is generated by this template file via make bin
command.
It may be good to add notes about this to the head of template file.
The `bin/iced` file is made (`make bin`) from this template. Previous commit missed to update this source.
Hi, thanks for the reminder, I have changed the template, too. Feel uncertain to add note to template, because someone didn't notice the file like me, will also overlook the note. |
@bootleq Thank you!
If we add notes to the template and run So it should help to avoid overlooking. |
Understand, how about this comment?
|
It might be helpful to specify what action to take if someone want to make a change.
|
This would give further contributor a hint, must make change to template.
Thanks, I have added the comment, hope further contributors will find those files were changed together. |
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.
@bootleq Thank you so much! LGTM
Hi, when using
iced repl
command,sometimes I need to use different aliases with clj
-M
exec option,for example
iced repl -M:xx:yy:iced
,but it produces another
-M
inMAIN_FLAG
handling and makes the prior no-effect.This PR try to detect
-M
in command line options and avoid second-M
as an fix.