{"thread":{"id":"44669","subject":"[PATCH 1/2] mergetools/kompare: simplify can_merge() by using \"false\"","startedAt":"2016-12-10T03:21:57Z","lastAt":"2016-12-12T11:45:06Z","messageCount":5,"participants":["David Aguilar","Johannes Sixt","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"307413","messageId":"20161210032144.25503-1-davvid@gmail.com","threadId":"44669","inReplyTo":null,"subject":"[PATCH 1/2] mergetools/kompare: simplify can_merge() by using \"false\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-12-10T03:21:43Z","receivedAt":"2016-12-10T03:21:57Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Signed-off-by: David Aguilar <davvid@gmail.com>\n---\nThis patch builds upon da/mergetool-trust-exit-code\n\n mergetools/kompare | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/mergetools/kompare b/mergetools/kompare\nindex e8c0bfa678..321022500b 100644\n--- a/mergetools/kompare\n+++ b/mergetools/kompare\n@@ -1,5 +1,5 @@\n can_merge () {\n-\treturn 1\n+\tfalse\n }\n \n diff_cmd () {\n-- \n2.11.0.27.gdeff8c7\n\n"},{"id":"307414","messageId":"20161210032144.25503-2-davvid@gmail.com","threadId":"44669","inReplyTo":"20161210032144.25503-1-davvid@gmail.com","subject":"[PATCH 2/2] mergetools/tortoisemerge: simplify can_diff() by using \"false\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-12-10T03:21:44Z","receivedAt":"2016-12-10T03:22:00Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Signed-off-by: David Aguilar <davvid@gmail.com>\n---\nThis patch builds upon da/mergetool-trust-exit-code\n\n mergetools/tortoisemerge | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge\nindex d7ab666a59..9067d8a4e5 100644\n--- a/mergetools/tortoisemerge\n+++ b/mergetools/tortoisemerge\n@@ -1,5 +1,5 @@\n can_diff () {\n-\treturn 1\n+\tfalse\n }\n \n merge_cmd () {\n-- \n2.11.0.27.gdeff8c7\n\n"},{"id":"307416","messageId":"37d8bc43-9f24-b8e8-cb52-de9cc9b2adde@kdbg.org","threadId":"44669","inReplyTo":"20161210032144.25503-2-davvid@gmail.com","subject":"Re: [PATCH 2/2] mergetools/tortoisemerge: simplify can_diff() by using \"false\"","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-12-10T08:15:34Z","receivedAt":"2016-12-10T08:15:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.12.2016 um 04:21 schrieb David Aguilar:\n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n> This patch builds upon da/mergetool-trust-exit-code\n>\n>  mergetools/tortoisemerge | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge\n> index d7ab666a59..9067d8a4e5 100644\n> --- a/mergetools/tortoisemerge\n> +++ b/mergetools/tortoisemerge\n> @@ -1,5 +1,5 @@\n>  can_diff () {\n> -\treturn 1\n> +\tfalse\n>  }\n\nWhy is this a simplification?\n\nMy concern is that 'false' is not necessarily a shell built-in. Then \nthis is actually a pessimization.\n\n-- Hannes\n\n"},{"id":"307489","messageId":"20161212071646.5bqnnjpfnmnj6fm4@gmail.com","threadId":"44669","inReplyTo":"37d8bc43-9f24-b8e8-cb52-de9cc9b2adde@kdbg.org","subject":"Re: [PATCH 2/2] mergetools/tortoisemerge: simplify can_diff() by using \"false\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-12-12T07:16:46Z","receivedAt":"2016-12-12T07:16:58Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, Dec 10, 2016 at 09:15:34AM +0100, Johannes Sixt wrote:\n> Am 10.12.2016 um 04:21 schrieb David Aguilar:\n> > Signed-off-by: David Aguilar <davvid@gmail.com>\n> > ---\n> > This patch builds upon da/mergetool-trust-exit-code\n> > \n> >  mergetools/tortoisemerge | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge\n> > index d7ab666a59..9067d8a4e5 100644\n> > --- a/mergetools/tortoisemerge\n> > +++ b/mergetools/tortoisemerge\n> > @@ -1,5 +1,5 @@\n> >  can_diff () {\n> > -\treturn 1\n> > +\tfalse\n> >  }\n> \n> Why is this a simplification?\n> \n> My concern is that 'false' is not necessarily a shell built-in. Then this is\n> actually a pessimization.\n\nThe \"simplification\" is semantic only.\n\nMotivation: if someone reads the implementation of can_diff()\nand it says \"false\" then that communicates intent moreso than\nreading \"return 1\", which a programmer unfamiliar with shell\nconventions might misinterpret as boolean \"true\".\n\nI care less about semantics then I do about making things better\nfor Windows, so we can forget about these two patches.\n-- \nDavid\n"},{"id":"307504","messageId":"775B98CA998147CC9A68CF6040B86CE1@PhilipOakley","threadId":"44669","inReplyTo":"20161212071646.5bqnnjpfnmnj6fm4@gmail.com","subject":"Re: [PATCH 2/2] mergetools/tortoisemerge: simplify can_diff() by using \"false\"","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2016-12-12T11:45:06Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"David Aguilar\" <davvid@gmail.com>\n> On Sat, Dec 10, 2016 at 09:15:34AM +0100, Johannes Sixt wrote:\n>> Am 10.12.2016 um 04:21 schrieb David Aguilar:\n>> > Signed-off-by: David Aguilar <davvid@gmail.com>\n>> > ---\n>> > This patch builds upon da/mergetool-trust-exit-code\n>> >\n>> >  mergetools/tortoisemerge | 2 +-\n>> >  1 file changed, 1 insertion(+), 1 deletion(-)\n>> >\n>> > diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge\n>> > index d7ab666a59..9067d8a4e5 100644\n>> > --- a/mergetools/tortoisemerge\n>> > +++ b/mergetools/tortoisemerge\n>> > @@ -1,5 +1,5 @@\n>> >  can_diff () {\n>> > - return 1\n>> > + false\n>> >  }\n>>\n>> Why is this a simplification?\n>>\n>> My concern is that 'false' is not necessarily a shell built-in. Then this \n>> is\n>> actually a pessimization.\n>\n> The \"simplification\" is semantic only.\n>\n> Motivation: if someone reads the implementation of can_diff()\n> and it says \"false\" then that communicates intent moreso than\n> reading \"return 1\", which a programmer unfamiliar with shell\n> conventions might misinterpret as boolean \"true\".\n>\n\nIs this a case where a short comment would be informative?\n\n + return 1 /* shell: false */\n\n\n> I care less about semantics then I do about making things better\n> for Windows, so we can forget about these two patches.\n> -- \n> David\n>\n--\nPhilip \n\n"}]}