-
Notifications
You must be signed in to change notification settings - Fork 127
Two small additions #65
base: stable
Are you sure you want to change the base?
Conversation
@@ -9,6 +9,6 @@ elif [ "$1" = "-v" ] || [ "$1" = "--version" ]; then | |||
$YCM_CONFIG_GEN_CC_PASSTHROUGH $@ | |||
|
|||
else | |||
echo "$@" >> $YCM_CONFIG_GEN_CC_LOG | |||
echo "YCM_CONFIG_GEN_PWD=$(pwd) $@" >> $YCM_CONFIG_GEN_CC_LOG |
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.
You need to quote the directory here, otherwise the parsing will fail if it contains spaces.
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.
Sorry for the delay in replying, real life has been quite hectic lately.
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.
Oops, I'll fix that. No worries about the delay!
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.
On second thought, while having spaces in a path does break this commit, I don't think adding quotes is the fix. YCM_CONFIG_GEN_PWD
is not a shell variable. YCM_CONFIG_GEN_PWD=<path>
is just a special word that gets created and parsed here. This is where the issue is, because there is no support for spaces in words :)
In addition to breaking this commit, I suspect that this limitation of words also prevents proper parsing of filename_flags with spaces in the path such as -I "a directory with spaces"
.
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.
The flags are split up / tokenized by split_flags()
, which uses unbalanced_quotes()
to concatenate adjacent words containing quotes. Since YCM_CONFIG_GEN_PWD=
is checked for in a flag, it will have the same logic applied to it.
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.
Ah I see. While that will work for YCM_CONFIG_GEN_PWD
, it seems that spaces in paths coming from the compiler invocation will still parse incorrectly since the quotes are dropped by $@
. I eagerly await your explanation of what I've missed this time ;)
2e8f06a is helpful when a CC command specifies a path relative to some directory other than the directory in which YCM-Generator is being executed.
8837813 adds support for
-D
usage that doesn't follow the format-D<name>=<value>
, namely usage such as-D<name>
. GCC allows this, as does clang.