~exec64/imv-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 3

[PATCH imv] title_text: remove trailing whitespace typo

Details
Message ID
<105c7-61dd7000-e3-4fc9cf00@191689967>
DKIM signature
missing
Download raw message
Patch: +1 -1
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 {

[imv/patches] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CH2TSKB6E19R.3KCZ434L69DAX@cirno>
In-Reply-To
<105c7-61dd7000-e3-4fc9cf00@191689967> (view parent)
DKIM signature
missing
Download raw message
imv/patches: SUCCESS in 3m59s

[title_text: remove trailing whitespace typo][0] from [Johannes Knoll][1]

[0]: https://lists.sr.ht/~exec64/imv-devel/patches/28160
[1]: Johannes.Knoll@hs-augsburg.de

✓ #669326 SUCCESS imv/patches/ubuntu.yml    https://builds.sr.ht/~exec64/job/669326
✓ #669324 SUCCESS imv/patches/fedora.yml    https://builds.sr.ht/~exec64/job/669324
✓ #669325 SUCCESS imv/patches/debian.yml    https://builds.sr.ht/~exec64/job/669325
✓ #669327 SUCCESS imv/patches/archlinux.yml https://builds.sr.ht/~exec64/job/669327
Details
Message ID
<2f92e0de-32c5-6ddf-cbf4-c03d3e3aa80f@harry.pm>
In-Reply-To
<105c7-61dd7000-e3-4fc9cf00@191689967> (view parent)
DKIM signature
pass
Download raw message
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 {
> 
Details
Message ID
<CKDYIMDKY218.2TY4AKEJJIZC0@arch>
In-Reply-To
<2f92e0de-32c5-6ddf-cbf4-c03d3e3aa80f@harry.pm> (view parent)
DKIM signature
pass
Download raw message
> 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?
Details
Message ID
<CKGIWI8M9EQI.1R9QLTG7I4IZF@arch>
In-Reply-To
<2f92e0de-32c5-6ddf-cbf4-c03d3e3aa80f@harry.pm> (view parent)
DKIM signature
pass
Download raw message
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.
Reply to thread Export thread (mbox)