threads / patch / 46201

patchmergetools/meld: improve backwards-compatibiilty when using "--output"

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

## tl;dr

6 messages between Jun 17, 2017 and Jun 19, 2017. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

David Aguilar· Jun 17, 2017, 10:24 UTC · lore

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 <file>" or "--output=<file>".

Use "--output=<file>" for better compatibility.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 mergetools/meld | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to mergetools/meld +1 −1
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
Samuel Lijin· Jun 17, 2017, 14:11 UTC · re: David Aguilar · lore

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

On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar <davvid@gmail.com> wrote:
Show 6 quoted lines
> 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 <file>" or
> "--output=<file>".
Do older versions also support both?
Show 23 quoted lines
> Use "--output=<file>" for better compatibility.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>  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
>
David Aguilar· Jun 18, 2017, 07:46 UTC · re: Samuel Lijin · lore

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

On Sat, Jun 17, 2017 at 10:11:36AM -0400, Samuel Lijin wrote:
Show 7 quoted lines
> On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar <davvid@gmail.com> 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 <file>" or
> > "--output=<file>".

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 <filename>" 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
Samuel Lijin· Jun 18, 2017, 09:11 UTC · re: David Aguilar · lore

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

On Sun, Jun 18, 2017 at 3:46 AM, David Aguilar <davvid@gmail.com> wrote:
Show 22 quoted lines
> On Sat, Jun 17, 2017 at 10:11:36AM -0400, Samuel Lijin wrote:
>> On Sat, Jun 17, 2017 at 6:24 AM, David Aguilar <davvid@gmail.com> 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 <file>" or
>> > "--output=<file>".
>
> 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 <filename>" 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.

David Aguilar· Jun 19, 2017, 00:17 UTC · re: Samuel Lijin · lore

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

On Sun, Jun 18, 2017 at 05:11:48AM -0400, Samuel Lijin wrote:
Show 26 quoted lines
> On Sun, Jun 18, 2017 at 3:46 AM, David Aguilar <davvid@gmail.com> 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 <davvid@gmail.com> 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 <file>" or
> >> > "--output=<file>".
> >
> > 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 <filename>" 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 <filename>
After the patch:
	--output=<filename>

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
Samuel Lijin· Jun 19, 2017, 01:01 UTC · re: David Aguilar · lore

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

On Sun, Jun 18, 2017 at 8:17 PM, David Aguilar <davvid@gmail.com> wrote:
Show 47 quoted lines
> On Sun, Jun 18, 2017 at 05:11:48AM -0400, Samuel Lijin wrote:
>> On Sun, Jun 18, 2017 at 3:46 AM, David Aguilar <davvid@gmail.com> 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 <davvid@gmail.com> 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 <file>" or
>> >> > "--output=<file>".
>> >
>> > 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 <filename>" 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 <filename>
>
> After the patch:
>
>         --output=<filename>
>
>
> 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 <filename>" though, that this does break.

← back to recent threads