[PATCH 0/3] mergetool-lib: Don't use deprecated variable to detect GNOME

[PATCH 0/3] mergetool-lib: Don't use deprecated variable to detect GNOME

[Marco Trevisan via GitGitGadget]

To list merge tool candidates we used to use a private GNOME env variable (
GNOME_DESKTOP_SESSION_ID) that has been deprecated for long time ago and
removed as part of GNOME 3.30.0 release [1].

So replace this using XDG_CURRENT_DESKTOP instead, and cleanup the code to
avoid duplication and supporting KDE's kdiff3 better.

[1] https://gitlab.gnome.org/GNOME/gnome-session/-/commit/00e0e6226371d53f65

Marco Trevisan (Treviño) (3):
  mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME
  mergetool-lib: keep a list of cross desktop merge tools
  mergetool-lib: give kdiff3 prioirty in KDE environments

 git-mergetool--lib.sh | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)


base-commit: 85b4e0a6dc8407de6f69808d9ee6debdf167ced3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-693%2F3v1n0%2Fdesktop-envs-fixes-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-693/3v1n0/desktop-envs-fixes-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/693
-- 
gitgitgadget

[PATCH 1/3] mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME

[Marco Trevisan (Treviño) via GitGitGadget]

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

To list merge tool candidates we used to use a private GNOME env
variable (GNOME_DESKTOP_SESSION_ID) that has been deprecated for long time ago
and removed as part of GNOME 3.30.0 release [1].

So, git should instead check the XDG_CURRENT_DESKTOP env variable, that
is supported by all the desktop environments.

Since the variable is actually a colon-separated list of names that the current
desktop is known as, we need to go through all the values to ensure
we're using GNOME.

[1] https://gitlab.gnome.org/GNOME/gnome-session/-/commit/00e0e6226371d53f65

Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
---
 git-mergetool--lib.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 204a5acd66..be28fe375f 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -266,6 +266,19 @@ run_merge_cmd () {
 	fi
 }
 
+is_desktop () {
+	IFS=':'
+	for desktop in ${XDG_CURRENT_DESKTOP}
+	do
+		if test "$desktop" = "$1"
+		then
+			return 0
+		fi
+	done
+
+	return 1
+}
+
 list_merge_tool_candidates () {
 	if merge_mode
 	then
@@ -275,7 +288,7 @@ list_merge_tool_candidates () {
 	fi
 	if test -n "$DISPLAY"
 	then
-		if test -n "$GNOME_DESKTOP_SESSION_ID"
+		if is_desktop "GNOME"
 		then
 			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
 		else
-- 
gitgitgadget

Re: [PATCH 1/3] mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME

[Eric Sunshine]

On Wed, Aug 5, 2020 at 3:51 PM Marco Trevisan (Treviño) via
GitGitGadget <gitgitgadget@gmail.com> wrote:
> To list merge tool candidates we used to use a private GNOME env
> variable (GNOME_DESKTOP_SESSION_ID) that has been deprecated for long time ago
> and removed as part of GNOME 3.30.0 release [1].
>
> So, git should instead check the XDG_CURRENT_DESKTOP env variable, that
> is supported by all the desktop environments.
>
> Since the variable is actually a colon-separated list of names that the current
> desktop is known as, we need to go through all the values to ensure
> we're using GNOME.
>
> Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
> ---
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> @@ -266,6 +266,19 @@ run_merge_cmd () {
> +is_desktop () {
> +       IFS=':'

We usually want to restore the value of IFS after we're done with it.
For instance:

    OLDIFS=$IFS
    IFS=:

and then restore it before returning:

    IFS=$OLDIFS

> +       for desktop in ${XDG_CURRENT_DESKTOP}
> +       do
> +               if test "$desktop" = "$1"
> +               then
> +                       return 0
> +               fi
> +       done
> +
> +       return 1
> +}

Rather than looping and mucking with IFS, even easier would be:

is_desktop () {
    case ":$XDG_CURRENT_DESKTOP:" in
    *:$1:*) return 0 ;;
    *) return 1 ;;
    esac
}

But perhaps that's too magical for people?

> @@ -275,7 +288,7 @@ list_merge_tool_candidates () {
> -               if test -n "$GNOME_DESKTOP_SESSION_ID"
> +               if is_desktop "GNOME"

Why do we need to retire the $GNOME_DESKTOP_SESSION_ID check here,
thus penalizing people who might still be on an old version of GNOME?
It doesn't seem like it would be a maintenance burden to continue
checking it while also taking advantage of $XDG_CURRENT_DESKTOP:

    if test -n "$GNOME_DESKTOP_SESSION_ID" || is_desktop GNOME

Re: [PATCH 1/3] mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Rather than looping and mucking with IFS, even easier would be:
>
> is_desktop () {
>     case ":$XDG_CURRENT_DESKTOP:" in
>     *:$1:*) return 0 ;;
>     *) return 1 ;;
>     esac
> }
>
> But perhaps that's too magical for people?

Nah, that is exactly how 'case' in shell is supposed to be used.

Thanks.

Re: [PATCH 1/3] mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME

[Marco Trevisan (Treviño)]

Hi,

Il giorno mer 5 ago 2020 alle ore 22:58 Eric Sunshine
<sunshine@sunshineco.com> ha scritto:
>
> On Wed, Aug 5, 2020 at 3:51 PM Marco Trevisan (Treviño) via
> GitGitGadget <gitgitgadget@gmail.com> wrote:
> > To list merge tool candidates we used to use a private GNOME env
> > variable (GNOME_DESKTOP_SESSION_ID) that has been deprecated for long time ago
> > and removed as part of GNOME 3.30.0 release [1].
> >
> > So, git should instead check the XDG_CURRENT_DESKTOP env variable, that
> > is supported by all the desktop environments.
> >
> > Since the variable is actually a colon-separated list of names that the current
> > desktop is known as, we need to go through all the values to ensure
> > we're using GNOME.
> >
> > Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
> > ---
> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> > @@ -266,6 +266,19 @@ run_merge_cmd () {
> > +is_desktop () {
> > +       IFS=':'
>
> We usually want to restore the value of IFS after we're done with it.
> For instance:
>
>     OLDIFS=$IFS
>     IFS=:
>
> and then restore it before returning:
>
>     IFS=$OLDIFS

Yeah, that's true and always a good practice, but it was never
happening in this file, so I followed the style.

> > +       for desktop in ${XDG_CURRENT_DESKTOP}
> > +       do
> > +               if test "$desktop" = "$1"
> > +               then
> > +                       return 0
> > +               fi
> > +       done
> > +
> > +       return 1
> > +}
>
> Rather than looping and mucking with IFS, even easier would be:
>
> is_desktop () {
>     case ":$XDG_CURRENT_DESKTOP:" in
>     *:$1:*) return 0 ;;
>     *) return 1 ;;
>     esac
> }
>
> But perhaps that's too magical for people?

Ok, fair enough, this is fine as well.

> > @@ -275,7 +288,7 @@ list_merge_tool_candidates () {
> > -               if test -n "$GNOME_DESKTOP_SESSION_ID"
> > +               if is_desktop "GNOME"
>
> Why do we need to retire the $GNOME_DESKTOP_SESSION_ID check here,
> thus penalizing people who might still be on an old version of GNOME?
> It doesn't seem like it would be a maintenance burden to continue
> checking it while also taking advantage of $XDG_CURRENT_DESKTOP:
>
>     if test -n "$GNOME_DESKTOP_SESSION_ID" || is_desktop GNOME

Ok, keeping this as well, it's even true that most people won't be in
gnome 2 these days, but not a problem to keep it around.

-- 
Treviño's World - Life and Linux
http://www.3v1n0.net

[PATCH 2/3] mergetool-lib: keep a list of cross desktop merge tools

[Marco Trevisan (Treviño) via GitGitGadget]

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

Instead of repeating the same tools multiple times, let's just keep them
in another variable and list them depending the current desktop

Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
---
 git-mergetool--lib.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index be28fe375f..243cd2b06b 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -288,11 +288,12 @@ list_merge_tool_candidates () {
 	fi
 	if test -n "$DISPLAY"
 	then
+		cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
 		if is_desktop "GNOME"
 		then
-			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
+			tools="meld $cross_desktop_tools $tools"
 		else
-			tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
+			tools="$cross_desktop_tools meld $tools"
 		fi
 		tools="$tools gvimdiff diffuse diffmerge ecmerge"
 		tools="$tools p4merge araxis bc codecompare"
-- 
gitgitgadget

Re: [PATCH 2/3] mergetool-lib: keep a list of cross desktop merge tools

[Eric Sunshine]

On Wed, Aug 5, 2020 at 3:53 PM Marco Trevisan (Treviño) via
GitGitGadget <gitgitgadget@gmail.com> wrote:
> Instead of repeating the same tools multiple times, let's just keep them
> in another variable and list them depending the current desktop
>
> Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
> ---
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> @@ -288,11 +288,12 @@ list_merge_tool_candidates () {
>         if test -n "$DISPLAY"
>         then
> +               cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
>                 if is_desktop "GNOME"
>                 then
> -                       tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
> +                       tools="meld $cross_desktop_tools $tools"
>                 else
> -                       tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
> +                       tools="$cross_desktop_tools meld $tools"
>                 fi

I have mixed feelings about this change. On the one hand, I see the
reason for doing it if the list of tools remains substantially the
same in each case, but it also seems like it could become a burden,
possibly requiring factoring the list into more pieces, as new
platforms or tools are added.

What I might find more compelling is creation of a table of tools and
the platforms for which they are suitable. It doesn't seem like it
would be too difficult to express such a table in shell and to extract
the desired rows (but that might be overkill). At any rate, I'm rather
"meh" on this change, though I don't oppose it strongly.

Re: [PATCH 2/3] mergetool-lib: keep a list of cross desktop merge tools

[Johannes Sixt]

Am 05.08.20 um 23:08 schrieb Eric Sunshine:
> On Wed, Aug 5, 2020 at 3:53 PM Marco Trevisan (Treviño) via
> GitGitGadget <gitgitgadget@gmail.com> wrote:
>> Instead of repeating the same tools multiple times, let's just keep them
>> in another variable and list them depending the current desktop
>>
>> Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
>> ---
>> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
>> @@ -288,11 +288,12 @@ list_merge_tool_candidates () {
>>         if test -n "$DISPLAY"
>>         then
>> +               cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
>>                 if is_desktop "GNOME"
>>                 then
>> -                       tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
>> +                       tools="meld $cross_desktop_tools $tools"
>>                 else
>> -                       tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
>> +                       tools="$cross_desktop_tools meld $tools"
>>                 fi
> 
> I have mixed feelings about this change. On the one hand, I see the
> reason for doing it if the list of tools remains substantially the
> same in each case, but it also seems like it could become a burden,
> possibly requiring factoring the list into more pieces, as new
> platforms or tools are added.
> 
> What I might find more compelling is creation of a table of tools and
> the platforms for which they are suitable. It doesn't seem like it
> would be too difficult to express such a table in shell and to extract
> the desired rows (but that might be overkill). At any rate, I'm rather
> "meh" on this change, though I don't oppose it strongly.

There may be some confusion caused by the name of the new variable. A
better name would perhaps be $tools_with_desktop_dependent_preference.
(That's a tongue-in-cheek suggestion, BTW.)

On a side note, the refactoring that happened in 21d0ba7ebb0c
("difftool/mergetool: refactor commands to use git-mergetool--lib",
2009-04-08) did not preserve the original order of diff tools. The
spirit was to prefer meld over kompare and kdiff3 with GNOME, and the
other way round with KDE. But since that refactoring, kompare is always
first in the list.

-- Hannes

Re: [PATCH 2/3] mergetool-lib: keep a list of cross desktop merge tools

[Marco Trevisan (Treviño)]

Hi,

Il giorno gio 6 ago 2020 alle ore 10:16 Johannes Sixt <j6t@kdbg.org> ha scritto:
>
> On a side note, the refactoring that happened in 21d0ba7ebb0c
> ("difftool/mergetool: refactor commands to use git-mergetool--lib",
> 2009-04-08) did not preserve the original order of diff tools. The
> spirit was to prefer meld over kompare and kdiff3 with GNOME, and the
> other way round with KDE. But since that refactoring, kompare is always
> first in the list.

Well, actually when DISPLAY is set so basically always (as even under
WAYLAND that's defined for XWayland apps) meld is still preferred.

I'm not sure it makes sense to have tortoisemerge or kompare if no
display is set though, but I didn't want to change the whole thing.

-- 
Treviño's World - Life and Linux
http://www.3v1n0.net

Re: [PATCH 2/3] mergetool-lib: keep a list of cross desktop merge tools

[Marco Trevisan (Treviño)]

Il giorno mer 5 ago 2020 alle ore 23:08 Eric Sunshine
<sunshine@sunshineco.com> ha scritto:
>
> On Wed, Aug 5, 2020 at 3:53 PM Marco Trevisan (Treviño) via
> GitGitGadget <gitgitgadget@gmail.com> wrote:
> > Instead of repeating the same tools multiple times, let's just keep them
> > in another variable and list them depending the current desktop
> >
> > Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
> > ---
> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> > @@ -288,11 +288,12 @@ list_merge_tool_candidates () {
> >         if test -n "$DISPLAY"
> >         then
> > +               cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
> >                 if is_desktop "GNOME"
> >                 then
> > -                       tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
> > +                       tools="meld $cross_desktop_tools $tools"
> >                 else
> > -                       tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
> > +                       tools="$cross_desktop_tools meld $tools"
> >                 fi
>
> I have mixed feelings about this change. On the one hand, I see the
> reason for doing it if the list of tools remains substantially the
> same in each case, but it also seems like it could become a burden,
> possibly requiring factoring the list into more pieces, as new
> platforms or tools are added.

I kind of agree on that, so it was mostly a proposal but I can withdraw it.

> What I might find more compelling is creation of a table of tools and
> the platforms for which they are suitable. It doesn't seem like it
> would be too difficult to express such a table in shell and to extract
> the desired rows (but that might be overkill). At any rate, I'm rather
> "meh" on this change, though I don't oppose it strongly.

Yeah, I was thinking the same, but also could be a bit complicated to
maintain especially when it comes using something that needs to be
supported by pure sh.

-- 
Treviño's World - Life and Linux
http://www.3v1n0.net

[PATCH 3/3] mergetool-lib: give kdiff3 prioirty in KDE environments

[Marco Trevisan (Treviño) via GitGitGadget]

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
---
 git-mergetool--lib.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 243cd2b06b..90de6ee884 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -288,12 +288,15 @@ list_merge_tool_candidates () {
 	fi
 	if test -n "$DISPLAY"
 	then
-		cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
+		cross_desktop_tools="opendiff tkdiff xxdiff"
 		if is_desktop "GNOME"
 		then
-			tools="meld $cross_desktop_tools $tools"
+			tools="meld $cross_desktop_tools kdiff3 $tools"
+		elif is_desktop "KDE"
+		then
+			tools="kdiff3 $cross_desktop_tools meld $tools"
 		else
-			tools="$cross_desktop_tools meld $tools"
+			tools="$cross_desktop_tools kdiff3 meld $tools"
 		fi
 		tools="$tools gvimdiff diffuse diffmerge ecmerge"
 		tools="$tools p4merge araxis bc codecompare"
-- 
gitgitgadget

Re: [PATCH 3/3] mergetool-lib: give kdiff3 prioirty in KDE environments

[Eric Sunshine]

On Wed, Aug 5, 2020 at 4:02 PM Marco Trevisan (Treviño) via
GitGitGadget <gitgitgadget@gmail.com> wrote:
> mergetool-lib: give kdiff3 prioirty in KDE environments

s/prioirty/priority/

> Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
> ---
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> @@ -288,12 +288,15 @@ list_merge_tool_candidates () {
> -               cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
> +               cross_desktop_tools="opendiff tkdiff xxdiff"
>                 if is_desktop "GNOME"
>                 then
> -                       tools="meld $cross_desktop_tools $tools"
> +                       tools="meld $cross_desktop_tools kdiff3 $tools"
> +               elif is_desktop "KDE"
> +               then
> +                       tools="kdiff3 $cross_desktop_tools meld $tools"
>                 else
> -                       tools="$cross_desktop_tools meld $tools"
> +                       tools="$cross_desktop_tools kdiff3 meld $tools"
>                 fi

Wouldn't this change the behavior for people running old KDE which
doesn't have XDG_CURRENT_DESKTOP, giving "kdiff3" much lower priority
than it had before?

This change also illustrates why I wasn't convinced that patch 2/3 was
necessarily a good idea. When you touch 'cross_desktop_tools' here,
you end up having to touch all the other 'tools=' lines anyhow, so the
introduction of 'cross_desktop_tools' didn't buy much in terms of
reduced maintenance cost.

Re: [PATCH 3/3] mergetool-lib: give kdiff3 prioirty in KDE environments

[Marco Trevisan (Treviño)]

Il giorno mer 5 ago 2020 alle ore 23:16 Eric Sunshine
<sunshine@sunshineco.com> ha scritto:
>
> On Wed, Aug 5, 2020 at 4:02 PM Marco Trevisan (Treviño) via
> GitGitGadget <gitgitgadget@gmail.com> wrote:
> > mergetool-lib: give kdiff3 prioirty in KDE environments
>
> s/prioirty/priority/
>
> > Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
> > ---
> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> > @@ -288,12 +288,15 @@ list_merge_tool_candidates () {
> > -               cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
> > +               cross_desktop_tools="opendiff tkdiff xxdiff"
> >                 if is_desktop "GNOME"
> >                 then
> > -                       tools="meld $cross_desktop_tools $tools"
> > +                       tools="meld $cross_desktop_tools kdiff3 $tools"
> > +               elif is_desktop "KDE"
> > +               then
> > +                       tools="kdiff3 $cross_desktop_tools meld $tools"
> >                 else
> > -                       tools="$cross_desktop_tools meld $tools"
> > +                       tools="$cross_desktop_tools kdiff3 meld $tools"
> >                 fi
>
> Wouldn't this change the behavior for people running old KDE which
> doesn't have XDG_CURRENT_DESKTOP, giving "kdiff3" much lower priority
> than it had before?

Yeah, true.. So to avoid this we can just also check for
KDE_FULL_SESSION, that has been introduced by KDE 3.2, and this should
be enough I think.

> This change also illustrates why I wasn't convinced that patch 2/3 was
> necessarily a good idea. When you touch 'cross_desktop_tools' here,
> you end up having to touch all the other 'tools=' lines anyhow, so the
> introduction of 'cross_desktop_tools' didn't buy much in terms of
> reduced maintenance cost.

Yeah, I had the same feeling, TBH.

-- 
Treviño's World - Life and Linux
http://www.3v1n0.net

Re: [PATCH 3/3] mergetool-lib: give kdiff3 prioirty in KDE environments

[Eric Sunshine]

On Thu, Aug 6, 2020 at 8:37 AM Marco Trevisan (Treviño) <mail@3v1n0.net> wrote:
> Il giorno mer 5 ago 2020 alle ore 23:16 Eric Sunshine
> <sunshine@sunshineco.com> ha scritto:
> > Wouldn't this change the behavior for people running old KDE which
> > doesn't have XDG_CURRENT_DESKTOP, giving "kdiff3" much lower priority
> > than it had before?
>
> Yeah, true.. So to avoid this we can just also check for
> KDE_FULL_SESSION, that has been introduced by KDE 3.2, and this should
> be enough I think.

I'm not a user of git-mergetool or KDE, so I can't speak as an
end-user. My comment was made merely as a reviewer of the code.

If it is easy to avoid the behavior change by also checking
KDE_FULL_SESSION in addition to the new check of XDG_CURRENT_DESKTOP
without it being a maintenance burden, then that sounds like a good
choice. On the other hand, if there wasn't a good way to avoid
changing behavior for users of older KDE, then explaining that in the
commit message would be expected.

[PATCH v2 0/2] mergetool-lib: Don't use deprecated variable to detect GNOME

[Marco Trevisan via GitGitGadget]

To list merge tool candidates we used to use a private GNOME env variable (
GNOME_DESKTOP_SESSION_ID) that has been deprecated for long time ago and
removed as part of GNOME 3.30.0 release [1].

So replace this using XDG_CURRENT_DESKTOP instead, and cleanup the code to
avoid duplication and supporting KDE's kdiff3 better.

[1] https://gitlab.gnome.org/GNOME/gnome-session/-/commit/00e0e6226371d53f65

Marco Trevisan (Treviño) (2):
  mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME
  mergetool-lib: give kdiff3 priority in KDE environments

 git-mergetool--lib.sh | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)


base-commit: 85b4e0a6dc8407de6f69808d9ee6debdf167ced3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-693%2F3v1n0%2Fdesktop-envs-fixes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-693/3v1n0/desktop-envs-fixes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/693

Range-diff vs v1:

 1:  66a026ae67 ! 1:  26b25b0d65 mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME
     @@ Commit message
          variable (GNOME_DESKTOP_SESSION_ID) that has been deprecated for long time ago
          and removed as part of GNOME 3.30.0 release [1].
      
     -    So, git should instead check the XDG_CURRENT_DESKTOP env variable, that
     -    is supported by all the desktop environments.
     +    So, git should instead primarily check the XDG_CURRENT_DESKTOP env variable,
     +    that is now supported by all the desktop environments.
      
          Since the variable is actually a colon-separated list of names that the current
     -    desktop is known as, we need to go through all the values to ensure
     -    we're using GNOME.
     +    desktop is known as, we need to check if the value is set if we're using GNOME.
      
          [1] https://gitlab.gnome.org/GNOME/gnome-session/-/commit/00e0e6226371d53f65
      
     @@ git-mergetool--lib.sh: run_merge_cmd () {
       }
       
      +is_desktop () {
     -+	IFS=':'
     -+	for desktop in ${XDG_CURRENT_DESKTOP}
     -+	do
     -+		if test "$desktop" = "$1"
     -+		then
     -+			return 0
     -+		fi
     -+	done
     -+
     -+	return 1
     ++	case ":$XDG_CURRENT_DESKTOP:" in
     ++	*:$1:*)
     ++		return 0
     ++		;;
     ++	*)
     ++		return 1
     ++		;;
     ++	esac
      +}
      +
       list_merge_tool_candidates () {
     @@ git-mergetool--lib.sh: list_merge_tool_candidates () {
       	if test -n "$DISPLAY"
       	then
      -		if test -n "$GNOME_DESKTOP_SESSION_ID"
     -+		if is_desktop "GNOME"
     ++		if is_desktop "GNOME" || test -n "$GNOME_DESKTOP_SESSION_ID"
       		then
       			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
       		else
 2:  fc0d2b103e < -:  ---------- mergetool-lib: keep a list of cross desktop merge tools
 3:  37090d2322 ! 2:  c18a5edf50 mergetool-lib: give kdiff3 prioirty in KDE environments
     @@ Metadata
      Author: Marco Trevisan (Treviño) <mail@3v1n0.net>
      
       ## Commit message ##
     -    mergetool-lib: give kdiff3 prioirty in KDE environments
     +    mergetool-lib: give kdiff3 priority in KDE environments
      
          Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
      
       ## git-mergetool--lib.sh ##
      @@ git-mergetool--lib.sh: list_merge_tool_candidates () {
     - 	fi
     - 	if test -n "$DISPLAY"
     - 	then
     --		cross_desktop_tools="opendiff kdiff3 tkdiff xxdiff"
     -+		cross_desktop_tools="opendiff tkdiff xxdiff"
     - 		if is_desktop "GNOME"
     + 		if is_desktop "GNOME" || test -n "$GNOME_DESKTOP_SESSION_ID"
       		then
     --			tools="meld $cross_desktop_tools $tools"
     -+			tools="meld $cross_desktop_tools kdiff3 $tools"
     -+		elif is_desktop "KDE"
     + 			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
     ++		elif is_desktop "KDE" || test x"$KDE_FULL_SESSION" = x"true"
      +		then
     -+			tools="kdiff3 $cross_desktop_tools meld $tools"
     ++			tools="kdiff3 opendiff tkdiff xxdiff meld $tools"
       		else
     --			tools="$cross_desktop_tools meld $tools"
     -+			tools="$cross_desktop_tools kdiff3 meld $tools"
     + 			tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
       		fi
     - 		tools="$tools gvimdiff diffuse diffmerge ecmerge"
     - 		tools="$tools p4merge araxis bc codecompare"

-- 
gitgitgadget

[PATCH v2 1/2] mergetool-lib: use $XDG_CURRENT_DESKTOP to check GNOME

[Marco Trevisan (Treviño) via GitGitGadget]

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

To list merge tool candidates we used to use a private GNOME env
variable (GNOME_DESKTOP_SESSION_ID) that has been deprecated for long time ago
and removed as part of GNOME 3.30.0 release [1].

So, git should instead primarily check the XDG_CURRENT_DESKTOP env variable,
that is now supported by all the desktop environments.

Since the variable is actually a colon-separated list of names that the current
desktop is known as, we need to check if the value is set if we're using GNOME.

[1] https://gitlab.gnome.org/GNOME/gnome-session/-/commit/00e0e6226371d53f65

Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
---
 git-mergetool--lib.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 204a5acd66..f9d8f309c8 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -266,6 +266,17 @@ run_merge_cmd () {
 	fi
 }
 
+is_desktop () {
+	case ":$XDG_CURRENT_DESKTOP:" in
+	*:$1:*)
+		return 0
+		;;
+	*)
+		return 1
+		;;
+	esac
+}
+
 list_merge_tool_candidates () {
 	if merge_mode
 	then
@@ -275,7 +286,7 @@ list_merge_tool_candidates () {
 	fi
 	if test -n "$DISPLAY"
 	then
-		if test -n "$GNOME_DESKTOP_SESSION_ID"
+		if is_desktop "GNOME" || test -n "$GNOME_DESKTOP_SESSION_ID"
 		then
 			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
 		else
-- 
gitgitgadget

[PATCH v2 2/2] mergetool-lib: give kdiff3 priority in KDE environments

[Marco Trevisan (Treviño) via GitGitGadget]

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
---
 git-mergetool--lib.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index f9d8f309c8..ac6695ea26 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -289,6 +289,9 @@ list_merge_tool_candidates () {
 		if is_desktop "GNOME" || test -n "$GNOME_DESKTOP_SESSION_ID"
 		then
 			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
+		elif is_desktop "KDE" || test x"$KDE_FULL_SESSION" = x"true"
+		then
+			tools="kdiff3 opendiff tkdiff xxdiff meld $tools"
 		else
 			tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
 		fi
-- 
gitgitgadget