Re: [vim/vim] configure: honor `--disable-hardcopy-pango` with GTK (PR #20945)

----==_mimepart_6a72d07964f16_100118032548a
Content-Type: text/plain; charset="UTF-8"

h-east left a comment (vim/vim#20945)

Thanks.
The fix itself is right, and splitting the "auto" default from an explicit
`--enable`/`--disable` is the correct shape.  Four points on the details.

### The generated `auto/configure` uses the wrong form

```sh
+else $as_nop
+  enable_hardcopy_pango="auto"
```

`$as_nop` is not defined in this `auto/configure` (0 occurrences; the
`case e in #(` form appears 425 times), so that `else` branch does not do what
it looks like it does.  Regenerating with autoconf 2.72 gives:

```sh
else case e in #(
  e) enable_hardcopy_pango="auto" ;;
esac
fi
```

If you regenerate rather than hand-edit, note that `make autoconf` also
rewrote four unrelated hunks in the `AC_SYS_LARGEFILE` block for me on the
same autoconf version.  Keeping only the hardcopy hunk is the safer route.

### Indentation in configure.ac

The `AC_ARG_ENABLE` block is re-indented from a tab to four spaces, while the
rest of `configure.ac` uses tabs.

### The root cause is missing from the commit message

The Problem/Solution text describes the symptom, but not why the default never
applied in the first place:

```m4
	,, enable_hardcopy_pango="no")
```

`AC_ARG_ENABLE` takes four arguments, so the extra comma put that default in a
fifth argument, where it was discarded.  That is why the generated `configure`
had no `else` branch, and why an unspecified `--enable-hardcopy-pango` left
the variable empty rather than "no".  Your patch fixes that too by moving it
to the action-if-not-given argument; worth saying so, since the old code was
broken in two ways rather than one.

### Handle --enable-fail-if-missing

With Pango or Cairo missing and an explicit `--enable-hardcopy-pango`, the
configure run silently continues with the feature off, which is what
`--enable-fail-if-missing` exists to prevent.  lua, perl, python and python3
all error out there:

```m4
  if test "$fail_if_missing" = "yes" -a "$enable_hardcopy_pango" = "yes"; then
    AC_MSG_ERROR([could not configure hardcopy-pango])
  fi
```

This belongs in this patch: before your change the option was discarded either
way, so there was nothing to honour and nothing to fail on.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/vim/vim/pull/20945#issuecomment-5188113655
You are receiving this because you are subscribed to this thread.

Message ID: <vim/vim/pull/20945/[email protected]>

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups "vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To view this discussion visit https://groups.google.com/d/msgid/vim_dev/vim/vim/pull/20945/c5188113655%40github.com.

----==_mimepart_6a72d07964f16_100118032548a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div style=3D"display: flex; flex-wrap: wrap; white-space: pre-wrap; align-=
items: center; "><img height=3D"20" width=3D"20" style=3D"border-radius:50%=
; margin-right: 4px;" decoding=3D"async" src=3D"https://avatars.githubuserc=
ontent.com/u/518808" /><strong>h-east</strong> left a comment <a href=3D"ht=
tps://github.com/vim/vim/pull/20945#issuecomment-5188113655">(vim/vim#20945=
)</a></div>
<p dir=3D"auto">Thanks.<br>
The fix itself is right, and splitting the "auto" default from an explicit<=
br>
<code class=3D"notranslate">--enable</code>/<code class=3D"notranslate">--d=
isable</code> is the correct shape.  Four points on the details.</p>
<h3 dir=3D"auto">The generated <code class=3D"notranslate">auto/configure</=
code> uses the wrong form</h3>
<div class=3D"highlight highlight-source-shell" dir=3D"auto"><pre class=3D"=
notranslate">+else <span class=3D"pl-smi">$as_nop</span>
+  enable_hardcopy_pango=3D<span class=3D"pl-s"><span class=3D"pl-pds">"</s=
pan>auto<span class=3D"pl-pds">"</span></span></pre></div>
<p dir=3D"auto"><code class=3D"notranslate">$as_nop</code> is not defined i=
n this <code class=3D"notranslate">auto/configure</code> (0 occurrences; th=
e<br>
<code class=3D"notranslate">case e in #(</code> form appears 425 times), so=
 that <code class=3D"notranslate">else</code> branch does not do what<br>
it looks like it does.  Regenerating with autoconf 2.72 gives:</p>
<div class=3D"highlight highlight-source-shell" dir=3D"auto"><pre class=3D"=
notranslate"><span class=3D"pl-k">else</span> <span class=3D"pl-k">case</sp=
an> e <span class=3D"pl-k">in</span> <span class=3D"pl-c"><span class=3D"pl=
-c">#</span>(</span>
  e) enable_hardcopy_pango=3D<span class=3D"pl-s"><span class=3D"pl-pds">"<=
/span>auto<span class=3D"pl-pds">"</span></span> ;;
<span class=3D"pl-k">esac</span>
<span class=3D"pl-k">fi</span></pre></div>
<p dir=3D"auto">If you regenerate rather than hand-edit, note that <code cl=
ass=3D"notranslate">make autoconf</code> also<br>
rewrote four unrelated hunks in the <code class=3D"notranslate">AC_SYS_LARG=
EFILE</code> block for me on the<br>
same autoconf version.  Keeping only the hardcopy hunk is the safer route.<=
/p>
<h3 dir=3D"auto">Indentation in configure.ac</h3>
<p dir=3D"auto">The <code class=3D"notranslate">AC_ARG_ENABLE</code> block =
is re-indented from a tab to four spaces, while the<br>
rest of <code class=3D"notranslate">configure.ac</code> uses tabs.</p>
<h3 dir=3D"auto">The root cause is missing from the commit message</h3>
<p dir=3D"auto">The Problem/Solution text describes the symptom, but not wh=
y the default never<br>
applied in the first place:</p>
<div class=3D"highlight highlight-source-m4" dir=3D"auto"><pre class=3D"not=
ranslate">	,, enable_hardcopy_pango=3D"no")</pre></div>
<p dir=3D"auto"><code class=3D"notranslate">AC_ARG_ENABLE</code> takes four=
 arguments, so the extra comma put that default in a<br>
fifth argument, where it was discarded.  That is why the generated <code cl=
ass=3D"notranslate">configure</code><br>
had no <code class=3D"notranslate">else</code> branch, and why an unspecifi=
ed <code class=3D"notranslate">--enable-hardcopy-pango</code> left<br>
the variable empty rather than "no".  Your patch fixes that too by moving i=
t<br>
to the action-if-not-given argument; worth saying so, since the old code wa=
s<br>
broken in two ways rather than one.</p>
<h3 dir=3D"auto">Handle --enable-fail-if-missing</h3>
<p dir=3D"auto">With Pango or Cairo missing and an explicit <code class=3D"=
notranslate">--enable-hardcopy-pango</code>, the<br>
configure run silently continues with the feature off, which is what<br>
<code class=3D"notranslate">--enable-fail-if-missing</code> exists to preve=
nt.  lua, perl, python and python3<br>
all error out there:</p>
<div class=3D"highlight highlight-source-m4" dir=3D"auto"><pre class=3D"not=
ranslate">  if test "$fail_if_missing" =3D "yes" -a "$enable_hardcopy_pango=
" =3D "yes"; then
    <span class=3D"pl-k">AC_MSG_ERROR</span><span class=3D"pl-pds">(</span>=
<span class=3D"pl-c1"><span class=3D"pl-pds">[</span>could not configure ha=
rdcopy-pango<span class=3D"pl-pds">]</span></span><span class=3D"pl-pds">)<=
/span>
  fi</pre></div>
<p dir=3D"auto">This belongs in this patch: before your change the option w=
as discarded either<br>
way, so there was nothing to honour and nothing to fail on.</p>

<p style=3D"font-size:small;-webkit-text-size-adjust:none;color:#666;">&mda=
sh;<br />Reply to this email directly, <a href=3D"https://github.com/vim/vi=
m/pull/20945#issuecomment-5188113655">view it on GitHub</a>, or <a href=3D"=
https://github.com/notifications/unsubscribe-auth/ACY5DGEG4ZSY3L6BNFRTD2T5I=
LD7TAVCNFSNUABEKJSXA33TNF2G64TZHM2DAOJZG42DQMR3JFZXG5LFHM2TANRXGEZTGNJXGOQX=
MAQ">unsubscribe</a>.<br />Triage notifications, keep track of coding agent=
 tasks and review pull requests on the go with GitHub Mobile for <a href=3D=
"https://github.com/notifications/mobile/ios/ACY5DGATQLNRTBO2MGT76DD5ILD7TA=
5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJYHAYTCMZWGU22M4TFM=
FZW63VKON2WE43DOJUWEZLEUVSXMZLOOSVGM33PORSXEX3JN5ZQ">iOS</a> and <a href=3D=
"https://github.com/notifications/mobile/android/ACY5DGFP74MJGYXXNHZU4HT5IL=
D7TA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJYHAYTCMZWGU22M=
4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSXGM33PORSXEX3BNZSHE33JMQ">Android</a>.=
 Download it today!
<br />You are receiving this because you are subscribed to this thread.<img=
 src=3D"https://github.com/notifications/beacon/ACY5DGC5Z7NPQLTTV3NI2PT5ILD=
7TBFCNFSM6AAAAAC434OUYCWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3=
TUL5UWJTYAAAAACNJ4KT32M4TFMFZW63VKON2WE43DOJUWEZLE.gif" height=3D"1" width=
=3D"1" alt=3D"" /><span style=3D"color: transparent; font-size: 0; display:=
 none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: =
0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span>&lt;vim/vi=
m/pull/20945/c5188113655</span><span>@</span><span>github</span><span>.</sp=
an><span>com&gt;</span></span></p>

<script type=3D"application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/vim/vim/pull/20945#issuecomment-5188113655",
"url": "https://github.com/vim/vim/pull/20945#issuecomment-5188113655",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>

<p></p>

-- <br />
-- <br />
You received this message from the &quot;vim_dev&quot; maillist.<br />
Do not top-post! Type your reply below the text you are replying to.<br />
For more information, visit <a href=3D"http://www.vim.org/maillist.php">htt=
p://www.vim.org/maillist.php</a><br />
<br />
--- <br />
You received this message because you are subscribed to the Google Groups &=
quot;vim_dev&quot; group.<br />
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to <a href=3D"mailto:[email protected]">vim_dev+uns=
[email protected]</a>.<br />
To view this discussion visit <a href=3D"https://groups.google.com/d/msgid/=
vim_dev/vim/vim/pull/20945/c5188113655%40github.com?utm_medium=3Demail&utm_=
source=3Dfooter">https://groups.google.com/d/msgid/vim_dev/vim/vim/pull/209=
45/c5188113655%40github.com</a>.<br />

----==_mimepart_6a72d07964f16_100118032548a--