Hi Johannes,
Thanks for the patch, though this isn't actually a typo. The whitespace
here is re-inserting the separator between each word after the word has
been expanded. This does result in an extra whitespace at the end of the
output, which could be addressed, but we do still need to keep the
whitespace between words.
Harry
On 2022-01-11 11:54, Johannes Knoll wrote:
> src/imv.c | 2 +-> 1 file changed, 1 insertion(+), 1 deletion(-)> > diff --git a/src/imv.c b/src/imv.c> index 8380a12..0f52df8 100644> --- a/src/imv.c> +++ b/src/imv.c> @@ -2002,7 +2002,7 @@ static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, cons> setenv("IFS", "", 1);> if (wordexp(format, &word, 0) == 0) {> for (size_t i = 0; i < word.we_wordc; ++i) {> - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]);> + len += snprintf(buf + len, buf_len - len, "%s", word.we_wordv[i]);> }> wordfree(&word);> } else {>
> Thanks for the patch, though this isn't actually a typo. The> whitespace here is re-inserting the separator between each word after> the word has been expanded.
Since we set IFS variable above to "", I think the whole for loop will
execute no more than once. Wordexp won't separate the expansion result
into words and will keep all the whitespace intact.
So I think the proposed patch will work correctly, but this piece of
code still needs to be modified for readability: the for loop can make
one think it can be executed multiple times. We could do something like
this in place of for loop to show that the we_wordc is always 0 or 1:
assert(word.we_wordc <= 1); // or replace assert by a comment
if (word.we_wordc == 1) {
strncpy(buf, word.we_wordv[0], buf_len);
} else {
buf[0] = '\0';
}
What do you think?
I was wrong, the IFS='' still produces multiple words. I'll send a new
patch that removes the trailing whitespace preserving the spaces between
the words.