Left panel - show inactive remotes

[RussKie]

Do you want to request a feature or report a bug?

feature

What is the current behavior?

The left panel displays the currently configured remotes.
GE has a feature that allows to disable a remote but keep the information available to the app.

What is the expected behavior?

Display disabled remotes in the tree, if any.
The node that contains the inactive remotes should be collapsed by default.

It could look like:

+ Branches
- Remotes (active)
  |- upstream
  |- fork
- Remotes (inactive)
  |- john
  |- jim
  |- jack

or

+ Branches
- Remotes
  |- active
  |  |- upstream
  |  |- fork
  |- inactive
  |  |- john
  |  |- jim
  |  |- jack

A context menu should allow to

• enable an inactive remote,
• disable an active remote

[gerhardol]

Useful for repo maintainers, but if this decreases the hurdle also for occasional developers to try out feature branches, much is gained.

An inactive remote can probably not list any branches. Should double click activate? Should double click of normal remotes deactivate then? If so should handling be as @amaiorano proposed for submodules?

The alternative to tabs is to be able to reorder in the left panel and keep the expansion state. For instance, I want submodules on the top.

Remotes should probably be on the same level as Branches (I would prefer to have inactive separate myself but have no good motivation).

[vbjay]

I wouldn't want double click to alter the active/inactive state of my remote. That would drive me BONKERS. Instead, open remote dialog with that remote selected.

Instead of separate nodes, Use different icons and maybe a tooltip to say active/inactive.

[amaiorano]

I didn't even know about this feature. Are disabled remotes a Git feature, or a GE feature?

In any case, given the current implementation, it would be easier to implement as:

- Remotes
  |- active
  |  |- upstream
  |  |- fork
  |- inactive
  |  |- john
  |  |- jim
  |  |- jack

Another option would be to not bother with "active", and have only the "inactive" parent node, so like:

- Remotes
  |- upstream
  |- fork
  |- inactive
  |  |- john
  |  |- jim
  |  |- jack

And as you said, in all cases, inactive would begin collapsed.

[amaiorano]

I wouldn't want double click to alter the active/inactive state of my remote. That would drive me BONKERS. Instead, open remote dialog with that remote selected.

Yes, I recommend that double-click open a dialog that asks if you want to activate an inactive one. This would be the same behaviour as right-clicking and selecting "Activate", which should be the first context menu action. This would mimic the existing double-click behavior on other nodes; e.g. double-click a branch or right-click "checkout" both result in a check out branch dialog.

[gerhardol]

I didn't even know about this feature. Are disabled remotes a Git feature, or a GE feature?

Really GE, inactive remotes are hidden in .git/config so Git do not care about them
So if I do not want to see your changes and deactivating them, I have a section like:

[-remote "amaiorano"]
        url = git://github.com/amaiorano/gitextensions.git
        fetch = +refs/heads/*:refs/remotes/amaiorano/*

Another option would be to not bother with "active", and have only the "inactive" parent node, so like:

Better

[vbjay]

Yes, I recommend that double-click open a dialog that asks if you want to activate an inactive one

No Not asking the user if they want to change the state. Show the remote dialog.

If I double click on your remote I have ( following left panel ROT work) I would expect the dialog to show letting me manage the remote including disabling it. If i double clicked upstream, then upstream would be selected.

I highlighted the button that disables/enables a remote.

[RussKie]

Useful for repo maintainers, but if this decreases the hurdle also for occasional developers to try out feature branches, much is gained.

I use this feature in many repos, as a team lead I need to check other devs' work.

I didn't even know about this feature. Are disabled remotes a Git feature, or a GE feature?

It is a GE-specific feature (#3615).

I wouldn't want double click to alter the active/inactive state of my remote. That would drive me BONKERS. Instead, open remote dialog with that remote selected.

Agree, I wouldn't like it either. Personally I would like to have the following options:

• Manage remotes... (default, double click)
• Activate the remote (if inactive)
• Activate the remote and fetch (if inactive)
• Deactivate the remote (if active)

[amaiorano]

So I started looking into this, and trying out the disable/enable remotes feature, and the first thing I noticed is that removing a remote has the side effect of also removing all of its refs (.git/refs/remotes/<remote_name> is gone), but enabling it again does not fetch refs from the remote. As a result, after disable/enable, you cannot perform any actions on the remote until you've fetched from it (e.g. you can't check out a branch in the checkout pulldown, etc.).

Should I try to fix that? If we don't auto-fetch enabled remote refs, then after enabling a remote, you'd have to remember to fetch it for it to appear in the left panel.

[gerhardol]

Should I try to fix that?

Leave it for now in this issue.
If you believe you have a solution, please discuss it.

[RussKie]

I couldn't find an easy solution, when enabling remotes you need to fetch them again... hence I want to have an "activate and fetch" context menu item

…

On Tue, Jan 1, 2019, 9:08 AM Gerhard Olsson ***@***.*** wrote: Should I try to fix that? Leave it for now in this issue. If you believe you have a solution, please discuss it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6015 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEMyXnG0z2b_5w8OGqm25OD3cgr2MhSaks5u-orugaJpZM4Zk6Sn> .

[amaiorano]

Okay, I'll look into it some more. Having the left panel list remotes without branches isnt currently supported, so that's what needs to be added. Then we can provide the context actions you want, @RussKie. Also note that we'd want to show active remotes with no branches as well, and allow fetching on them. Should be doable.

[RussKie]

Worth noting, that I often enable/disable remotes in batches (e.g. reviewing several PRs from different contributors), so in my case I need both "activate" and "activate+fetch" options.

[amaiorano]

As I continue exploring, it looks like GitModule.GetRefs has a bug in it. If you pass in "tags: false, branches: true" to only retrieve branches, it will not return remote branches, only local ones. However, if you pass in "tags: true, branches: true", you'll get local and remote branches (and tags). It boils down to the fact that it uses different ways to get the refs:

        public IReadOnlyList<IGitRef> GetRefs(bool tags = true, bool branches = true)
        {
            var refList = GetRefList();

            return ParseRefs(refList);

            string GetRefList()
            {
                if (tags && branches)
                {
                    return _gitExecutable.GetOutput("show-ref --dereference");
                }

                if (tags)
                {
                    return _gitExecutable.GetOutput("show-ref --tags");
                }

                if (branches)
                {
                    return _gitExecutable.GetOutput(@"for-each-ref --sort=-committerdate refs/heads/ --format=""%(objectname) %(refname)""");
                }

                return "";
            }
        }

When only branches is wanted, the function uses for-each-ref but specifies only the "refs/heads/" path, so we don't get the refs from "refs/remotes/". This is surprising, and should be fixed, but there are a few calls to this function that pass in "tags: false, branches: true" that we need to make sure work as expected. Or perhaps we need to extent the API to accept another bool parameter "remotes".

[gerhardol]

Check master for update GetRefs), changed just recently
But remotes: should be added too, it seems. (Maybe using a Mode as in #6002)

[RussKie]

it looks like GitModule.GetRefs has a bug in it

Please open a new issue so we can track it separately.

[amaiorano]

Screenshot time! Here's the current progress:

Enabled remotes without branches, as well as disabled remotes are now displayed:

Initially "Disabled" is collapsed:

If you expand it:

One thing to note, "fsharp" has no children. It is a remote with no fetched branches (yet). EDIT: Also note that it's not positioned alphabetically -- normally it would be in between "amaiorano" and "origin". I will fix that soon. Fixed.

Right-clicking an enabled remote:

Right-clicking a disabled remote:

I'm not sure what to do about icons. I can probably find something for Enable and Disable, but I'm not quite sure what to use for "Enable and fetch".

Performing "Enable", "Enable and fetch", and "Disable" all work as expected. Enable/Disable will refresh the UI. Enable and fetch shows the fetch dialog, and then the UI is refreshed. Works really well!

[amaiorano]

If you want to try my changes, check out the branch on my fork:
https://github.com/amaiorano/gitextensions/tree/amaiorano/left-panel-remotes-improvements

[RussKie]

Good stuff!

I suggest using the same terminology as we use in the Remotes dialog: "Activate" and "Deactivate" (or change the dialog to match the left panel).
Instead of "Disabled" - perhaps "Inactive"?
I also suggest use the same icons for the context menu as used in the dialog.
I don't particularly mind which way we chose to go as long as we are consistent.

As an observation - I think we need to rename "Manage remotes" to "Manage remotes..." to indicate this command opens a dialog

[RussKie]

Looks pretty cool, though inactive remotes aren't sorted:

We could probably enhance the UX - when choosing to manage a remote, the dialog should pre-select the chosen remote

[amaiorano]

I suggest using the same terminology as we use in the Remotes dialog: "Activate" and "Deactivate" (or change the dialog to match the left panel). Instead of "Disabled" - perhaps "Inactive"?

Done. In code, it's "enable/disable", but for the UI, it's "active/inactive". Decided to just remain consistent.

I also suggest use the same icons for the context menu as used in the dialog. I don't particularly mind which way we chose to go as long as we are consistent.

Done, though I don't have a special icon for "Activate and fetch", so current using the eye open icon for both:

Would probably need an eye open icon with the blue down arrow that we use for fetch.

As an observation - I think we need to rename "Manage remotes" to "Manage remotes..." to indicate this command opens a dialog

Yep, had noticed that too. Fixed as well.

Branch has been updated if you want to try it now.

EDIT:

We could probably enhance the UX - when choosing to manage a remote, the dialog should pre-select the chosen remote

This is already how it works, it seems.

[RussKie]

Tried it - very cool! Digging it.

I tried combining the eye and the arrow:

Not entirely happy with the result. Perhaps this item can remain icon-less...

[amaiorano]

Can you tell me how to add icons in GE? I'll give it a shot as well.

[RussKie]

There are few ways - you can do it via the VS Designer, or - manually I often do it manually, the designer tends to make "noisy" changes. If you search for any referensed image you should be able to trace it all the way through. - drop a file into Icons folder - add to resources files (2 places) - reference the image - celebrate 😎

…

On Sun, Jan 6, 2019, 4:34 PM Antonio Maiorano ***@***.*** wrote: Can you tell me how to add icons in GE? I'll give it a shot as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6015 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEMyXgKN5_uxBYwSyYJMRlFNinhogqyfks5vAYrtgaJpZM4Zk6Sn> .

[RussKie]

Please have a look at 4176d05 as an example, that's pretty much the extent of changes when adding or modifying an image

[RussKie]

Found an annoyance: whenever deactivating or activating a remote the graph refreshes and the "Remotes/Inactive" node folds.
Is it difficult to retain the state across refreshes?

[amaiorano]

Is it difficult to retain the state across refreshes?

I can easily special case it for the Inactive folder. Ideally, though, we'd remember the expanded state of the entire tree and attempt to reapply it. Need to think about the best way to do that. I want to wait, though, until my left panel pubsub change is merged, as it will affect how we approach this. But in the meantime, I'll special case this one.

[amaiorano]

Please have a look at 4176d05 as an example, that's pretty much the extent of changes when adding or modifying an image

Thanks!

[amaiorano]

I pushed a couple of commits. First, I added an icon for RemoteEnableAndFetch:

The other commit somewhat addresses remembering the Inactive node expanded state. I'll explain what I mean. I didn't add any special code yet; instead, I rely on a side-effect of RepoObjectsTree.Nodes.FillTreeViewNode, a function that attempts to recycle existing TreeViewNode instances when rebuilding the structure after the "Node" structure has changed (Node is the base class for node types, and a TreeViewNode instance is created for each, with its "Tag" member set to the Node instance it matches). When this function recycles a TreeViewNode, the expanded state of that node is "remembered".

If you look at the FillTreeViewNode code, it's robust against insertion of new nodes, but if a node is removed, it won't recycle nodes properly. This can and should be fixed to at least improve node recycling, but I don't think we can rely on node recycling to always correctly remember expanded/collapsed state. I think this side effect was accidental, and wasn't really noticed because the Branches tree specifically expands to the currently active branch.

So right now, if you activate an inactive remote, it will "remember" the expanded state; but when you deactive a remote, it won't and the "Inactive" node will be collapsed. I'll think some more about this, and see what I can do.

[RussKie]

Your icon looks way better than mine 👍

…

On Mon, Jan 7, 2019, 7:26 AM Antonio Maiorano ***@***.*** wrote: I pushed a couple of commits. First, I added an icon for RemoteEnableAndFetch: [image: image] <https://user-images.githubusercontent.com/6893883/50740934-7f637f00-11c4-11e9-9cd4-507c8170b903.png> The other commit *somewhat* addresses remembering the Inactive node expanded state. I'll explain what I mean. I didn't add any special code yet; instead, I rely on a side-effect of RepoObjectsTree.Nodes.FillTreeViewNode, a function that attempts to recycle existing TreeViewNode instances when rebuilding the structure after the "Node" structure has changed (Node is the base class for node types, and a TreeViewNode instance is created for each, with its "Tag" member set to the Node instance it matches). When this function recycles a TreeViewNode, the expanded state of that node is "remembered". If you look at the FillTreeViewNode code, it's robust against insertion of new nodes, but if a node is removed, it won't recycle nodes properly. This can and should be fixed to at least improve node recycling, but I don't think we can rely on node recycling to always correctly remember expanded/collapsed state. I think this side effect was accidental, and wasn't really noticed because the Branches tree specifically expands to the currently active branch. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6015 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEMyXmVYGpef676-h0jgxLw85QJMxMgrks5vAlv_gaJpZM4Zk6Sn> .

[amaiorano]

Your icon looks way better than mine 👍

Thanks :) Have a little photoshopping experience thanks to my game dev background ;)

[amaiorano]

Okay, I just pushed a commit that adds the ability for the left panel to remember and re-apply the expanded state of nodes. This change is generic, and not really required for this specific change (show inactive remotes and remotes without branches). If you want, I can split this out into its own PR. Let me know. If you want to see the commit: amaiorano@e18b7d9

Edit: The only thing left to do is double-click to open the Remotes dialog. Since Remote nodes can have children, we have the same issue as I have with my submodules ROT work (this was mentioned above). I you want, I can use the same solution I already implemented for submodules here right away (double-click no longer expands inner nodes); or we can wait until my submodules work is merged to later add double-click to open on remotes. Let me know!

[gerhardol]

or we can wait until my submodules work is merged to later add double-click to open on remotes. Let me know!

Most important is that it is consistent, I suggest you make a choice (it is likely the initial implementation that sticks).

[amaiorano]

Most important is that it is consistent, I suggest you make a choice (it is likely the initial implementation that sticks).

Okay, I'm going to bring in the change to disable expand/collapse on double-clicking nodes in the left panel. This will apply to all nodes. This will only apply to nodes that don't have special double-click behaviour.

[amaiorano]

Okay, latest version on my fork now has double-click to open Remotes dialog, along with the change to disable expand/collapse on any inner node that overrides OnDoubleClick (like Visual Studio does in Solution Explorer).

@RussKie can you test out this latest version and let me know if it fits the bill? I've implemented pretty much everything mentioned on this thread. If you're happy with it, I'll clean up the commits and open a PR.

[RussKie]

Love it!

[amaiorano]

Love it!

Great! Will create PR soon.