Re: [PATCH] mergetools/meld: improve backwards-compatibiilty when using "--output"

[Samuel Lijin]

On Sun, Jun 18, 2017 at 8:17 PM, David Aguilar  wrote:
> On Sun, Jun 18, 2017 at 05:11:48AM -0400, Samuel Lijin wrote:
>> On Sun, Jun 18, 2017 at 3:46 AM, David Aguilar  wrote:
>> > On Sat, Jun 17, 2017 at 10:11:36AM -0400, Samuel Lijin wrote:
>> >> On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar  wrote:
>> >> > Meld 3.16.0 requires a "=" in the --output argument, as it uses
>> >> > a simple hand-rolled command-line parser.
>> >> >
>> >> > Newer versions of Meld (3.16.4, and possibly earlier) use
>> >> > optpaarse, which accepts either "--output " or
>> >> > "--output=".
>> >
>> > Junio, there's an optpaarse -> optparse typo in the commit message
>> > here in case you want to fix that up.
>> >
>> >>
>> >> Do older versions also support both?
>> >
>> > No.  When the "--output" option was first added (3.16.0, or possibly
>> > earlier) it used the simpler parser that does not undertand the
>> > "--output " form.
>> >
>> > Much older versions didn't support "--output" at all, so we don't have
>> > to worry about them since we already use the "--output" flag
>> > selectively based on whether or not it's supported.
>>
>> It sounds like this patch would break versions of Meld that use the
>> hand-rolled parser, then.
>
> I don't think so.
>
> The whole point of this patch is to make it compatible with the
> hand-rolled parser.
>
> Before the patch:
>
> --output 
>
> After the patch:
>
> --output=
>
>
> The form with "=" (the latter one) is the one that's maximally
> compatible.
>
> Please re-read the commit message and patch to verify that this is
> indeed true.

Whoops, sorry, yes, you're right. It does still sound like there are
some versions of Meld in the middle that rely on "--output "
though, that this does break.

Re: [PATCH] mergetools/meld: improve backwards-compatibiilty when using "--output"

[David Aguilar]

On Sun, Jun 18, 2017 at 05:11:48AM -0400, Samuel Lijin wrote:
> On Sun, Jun 18, 2017 at 3:46 AM, David Aguilar  wrote:
> > On Sat, Jun 17, 2017 at 10:11:36AM -0400, Samuel Lijin wrote:
> >> On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar  wrote:
> >> > Meld 3.16.0 requires a "=" in the --output argument, as it uses
> >> > a simple hand-rolled command-line parser.
> >> >
> >> > Newer versions of Meld (3.16.4, and possibly earlier) use
> >> > optpaarse, which accepts either "--output " or
> >> > "--output=".
> >
> > Junio, there's an optpaarse -> optparse typo in the commit message
> > here in case you want to fix that up.
> >
> >>
> >> Do older versions also support both?
> >
> > No.  When the "--output" option was first added (3.16.0, or possibly
> > earlier) it used the simpler parser that does not undertand the
> > "--output " form.
> >
> > Much older versions didn't support "--output" at all, so we don't have
> > to worry about them since we already use the "--output" flag
> > selectively based on whether or not it's supported.
> 
> It sounds like this patch would break versions of Meld that use the
> hand-rolled parser, then.

I don't think so.

The whole point of this patch is to make it compatible with the
hand-rolled parser.

Before the patch:

--output 

After the patch:

--output=


The form with "=" (the latter one) is the one that's maximally
compatible.

Please re-read the commit message and patch to verify that this is
indeed true.
-- 
David

Re: [PATCH] mergetools/meld: improve backwards-compatibiilty when using "--output"

[Samuel Lijin]

On Sun, Jun 18, 2017 at 3:46 AM, David Aguilar  wrote:
> On Sat, Jun 17, 2017 at 10:11:36AM -0400, Samuel Lijin wrote:
>> On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar  wrote:
>> > Meld 3.16.0 requires a "=" in the --output argument, as it uses
>> > a simple hand-rolled command-line parser.
>> >
>> > Newer versions of Meld (3.16.4, and possibly earlier) use
>> > optpaarse, which accepts either "--output " or
>> > "--output=".
>
> Junio, there's an optpaarse -> optparse typo in the commit message
> here in case you want to fix that up.
>
>>
>> Do older versions also support both?
>
> No.  When the "--output" option was first added (3.16.0, or possibly
> earlier) it used the simpler parser that does not undertand the
> "--output " form.
>
> Much older versions didn't support "--output" at all, so we don't have
> to worry about them since we already use the "--output" flag
> selectively based on whether or not it's supported.

It sounds like this patch would break versions of Meld that use the
hand-rolled parser, then.

Re: [PATCH] mergetools/meld: improve backwards-compatibiilty when using "--output"

[David Aguilar]

On Sat, Jun 17, 2017 at 10:11:36AM -0400, Samuel Lijin wrote:
> On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar  wrote:
> > Meld 3.16.0 requires a "=" in the --output argument, as it uses
> > a simple hand-rolled command-line parser.
> >
> > Newer versions of Meld (3.16.4, and possibly earlier) use
> > optpaarse, which accepts either "--output " or
> > "--output=".

Junio, there's an optpaarse -> optparse typo in the commit message
here in case you want to fix that up.

> 
> Do older versions also support both?

No.  When the "--output" option was first added (3.16.0, or possibly
earlier) it used the simpler parser that does not undertand the
"--output " form.

Much older versions didn't support "--output" at all, so we don't have
to worry about them since we already use the "--output" flag
selectively based on whether or not it's supported.
-- 
David

Re: [PATCH] mergetools/meld: improve backwards-compatibiilty when using "--output"

[Samuel Lijin]

On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar  wrote:
> Meld 3.16.0 requires a "=" in the --output argument, as it uses
> a simple hand-rolled command-line parser.
>
> Newer versions of Meld (3.16.4, and possibly earlier) use
> optpaarse, which accepts either "--output " or
> "--output=".

Do older versions also support both?

> Use "--output=" for better compatibility.
>
> Signed-off-by: David Aguilar 
> ---
>  mergetools/meld | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mergetools/meld b/mergetools/meld
> index bc178e8882..7a08470f88 100644
> --- a/mergetools/meld
> +++ b/mergetools/meld
> @@ -10,7 +10,7 @@ merge_cmd () {
>
> if test "$meld_has_output_option" = true
> then
> -   "$merge_tool_path" --output "$MERGED" \
> +   "$merge_tool_path" --output="$MERGED" \
> "$LOCAL" "$BASE" "$REMOTE"
> else
> "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
> --
> 2.13.1.453.gc0395165f3
>

[PATCH] mergetools/meld: improve backwards-compatibiilty when using "--output"

[David Aguilar]

Meld 3.16.0 requires a "=" in the --output argument, as it uses
a simple hand-rolled command-line parser.

Newer versions of Meld (3.16.4, and possibly earlier) use
optpaarse, which accepts either "--output " or
"--output=".

Use "--output=" for better compatibility.

Signed-off-by: David Aguilar 
---
 mergetools/meld | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mergetools/meld b/mergetools/meld
index bc178e8882..7a08470f88 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -10,7 +10,7 @@ merge_cmd () {
 
if test "$meld_has_output_option" = true
then
-   "$merge_tool_path" --output "$MERGED" \
+   "$merge_tool_path" --output="$MERGED" \
"$LOCAL" "$BASE" "$REMOTE"
else
"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
-- 
2.13.1.453.gc0395165f3