Skip to content

Add control with last interactive Git outputs and traces #10307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Oct 25, 2024
Merged

Add control with last interactive Git outputs and traces #10307

merged 5 commits into from
Oct 25, 2024

Conversation

mstv
Copy link
Member

@mstv mstv commented Oct 29, 2022 •
edited
Loading

Fixes #10206

Proposed changes

  • FormBrowse: Add control which is filled with the output from the last 20 Git commands executed in FormStatus
    as well as with trace output
  • Show the control as Tab or docked in the lower left corner (GE needs to be restarted in order to bring changed settings into effect)
  • Hotkey: Ctrl+9 for toggling panel visibility and focusing

Screenshots

Before

image

After

image

image

Test methodology

  • manual

Test environment(s)

  • Git Extensions 33.33.33
  • Build 63a2b45
  • Git 2.38.1.windows.1
  • Microsoft Windows NT 10.0.19045.0
  • .NET 6.0.10
  • DPI 96dpi (no scaling)
  • Microsoft.WindowsDesktop.App Versions
    Microsoft.WindowsDesktop.App 3.1.21 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
    Microsoft.WindowsDesktop.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
    Microsoft.WindowsDesktop.App 6.0.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
    Microsoft.WindowsDesktop.App 6.0.10 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).

✒️ I contribute this code under The Developer Certificate of Origin.

@mstv mstv self-assigned this Oct 29, 2022
@mstv mstv added this to the vNext milestone Oct 30, 2022
@mstv mstv force-pushed the feature/10206_git_output_tab branch from 5cfe121 to 906017d Compare October 30, 2022 09:47
@mstv
Copy link
Member Author

mstv commented Oct 30, 2022

@PeterMinin, you could try https://ci.appveyor.com/project/gitextensions/gitextensions/builds/45223566/artifacts.

If it was not such complicated, I would rather place the git output in the lower left corner in order to have it visible always.

image

@gerhardol
Copy link
Member

Nice!

Changing hotkeys could be sensitive, but probably OK here...
Any commands with more than one output?
What is the effort to add to command log, also for other commands?
Should be added to settings, like console and gpg (and build status). Should be default off.
A context menu to copy would be nice too - ctrl-c works though.

@PeterMinin
Copy link

Thanks @mstv! This looks useful already. However my idea was to keep output from multiple commands, so that you could return to it after you've done something else. I think that would also mean writing the commands themselves, otherwise it would be hard to navigate.

I took a look at the code to see if it's easy to implement that way. Although FormStatus doesn't have access to the command executed, its child FormProcess does. I think it's ok to move the hook to FormProcess. And perhaps it's even better, because some uses of FormStatus, namely FormStatus.ShowErrorDialog, aren't for long-running operations. For example, as far as I understand, and error in the staging dialog (which I usually encounter if I try to stage specific lines but have a wrong encoding selected in the viewer) uses that form, which I don't think is useful to keep. What do you think?

Also I guess it would be nice to show some placeholder before there is output to show.

@mstv mstv force-pushed the feature/10206_git_output_tab branch from cc433c0 to 63a2b45 Compare November 13, 2022 19:44
@mstv
Copy link
Member Author

mstv commented Nov 13, 2022 •
edited
Loading

I have updated the description.

to be done?

  1. internal AppSetting for number of displayed results
  2. hide by default
  3. store splitter position
  4. DPI-awareness
  5. add toggle button to main toolbar or to Left Panel toolbar:
    image

@gerhardol
Copy link
Member

to be done?
internal AppSetting for number of displayed results

yes. Maybe ttal size too?

hide by default

Probably

  • store splitter position

Related to this PR?

  • add toggle button to main toolbar or to Left Panel toolbar:

I do not want that there, maybe context menus on the tabs (in addition to AppSettings, as for other tabs)

I also want:

  • Context menu in the tab, see GPG info, to select/copy text
  • Move the tab to rightmost position. Will not change existing shortcuts, also separate use.

No direct comments on the code.

@mstv mstv changed the title Add tab page with last Git output Add control with last Git outputs Nov 13, 2022
@mstv
Copy link
Member Author

mstv commented Nov 13, 2022

I have updated the description.

... but forgot to push the update button.
There is no tab anymore but a splitter added to the Left Panel.

see GPG info

Could you elaborate?

@gerhardol
Copy link
Member

... but forgot to push the update button. There is no tab anymore but a splitter added to the Left Panel.

I liked the tab better than this panel
It always occupies space, it is not often I want to view the contents.
With this, I would deactivate by default, rather have the info in Command Log.

Could you elaborate?

image

Compile error

image

Still no major code comments, I will review (likely approve) when we agree on the display.

@mstv mstv force-pushed the feature/10206_git_output_tab branch from 63a2b45 to 1932647 Compare November 13, 2022 22:05
@mstv
Copy link
Member Author

mstv commented Nov 13, 2022

I liked the tab better than this panel
It always occupies space, it is not often I want to view the contents.

I don't need it always - but if, switching tabs is a nuisance. It is becoming a killer feature for me. Toggle it using Ctrl+9. Tab can be implemented as an option.

Compile error

My bad. Removed.

@RussKie
Copy link
Member

RussKie commented Nov 14, 2022

I don't like it, sorry. It looks very untidy. I would have thought that we'll have a new tab of some kind....

Also, any new controls should be encapsulated as UserControls (or whatever) and not be coded directly in FormBrowse. We should be decoupling and encapsulating elements as much as possible.

@mstv mstv marked this pull request as draft November 14, 2022 19:28
@mstv
Copy link
Member Author

mstv commented Nov 14, 2022

I should have marked it as draft / PoC more clearly... Extraction to a dedicated control is planned for after getting a consensus on the UX.

The variant as Tab is available in the first commit. But believe me it is not very usable.

@vbjay
Copy link
Contributor

vbjay commented Nov 18, 2022

Tried it out

image

Didn't create a new line between commands.

Maybe we can take a page from visual studio. This would be a great thing for auto hide from bottom of window.

image
image

@RussKie
Copy link
Member

RussKie commented Nov 18, 2022

Expandable/movable windows would be great, but it'd either require 3rd party controls (💰💰💰) or custom implementations (⌛⌛⌛).

@RussKie RussKie modified the milestones: 4.1, vNext Apr 10, 2023
@RussKie RussKie modified the milestones: 4.2, vNext Oct 7, 2023
@mstv mstv force-pushed the feature/10206_git_output_tab branch from 1932647 to 484ebd4 Compare November 4, 2023 23:34
@mstv mstv marked this pull request as ready for review November 4, 2023 23:34
@mstv mstv force-pushed the feature/10206_git_output_tab branch 3 times, most recently from 5c7d681 to 80400ba Compare November 8, 2023 23:12
@gerhardol
Copy link
Member

gerhardol commented Nov 12, 2023 •
edited
Loading

I have been using a previous version of this for dome time. No direct comments when browsing this update.
I do not get any output when using this PR though. Hints?
image

Rebase on master to clear the merge conflicts?

Edit: Still fails for me.

@mstv mstv force-pushed the feature/10206_git_output_tab branch from 80400ba to 5563845 Compare November 12, 2023 23:16
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat(FormStatusOutputLog): Normalize line endings
d9e364f 
and vertical tabs

Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat: Add OutputHistoryControl and controllers
b1788ba 
Refs: gitextensions#10307
@mstv
Copy link
Member Author

mstv commented Oct 1, 2024

Can we finally merge this PR?
It is a nuisance to not have this feature when testing PRs.

@RussKie
Copy link
Member

RussKie commented Oct 5, 2024

Sorry, I can't bring myself to approve with this layout:
image

@mstv
Copy link
Member Author

mstv commented Oct 5, 2024

Sorry, I can't bring myself to approve with this layout:

Then use the tab, which is the default, or turn the entire feature off. I want it, and I am not forcing you to want it.
I am asking whether there are significant concerns about the code itself.

@mstv
Copy link
Member Author

mstv commented Oct 5, 2024

I have updated the screenshot in the PR description:

image

@gerhardol gerhardol mentioned this pull request Oct 16, 2024
Merged
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 20, 2024
@mstv
feat: Add SubscribableTraceListener
a227dac 
Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 20, 2024
@mstv
feat: Add OutputHistoryModel
75759a8 
Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 20, 2024
@mstv
feat: Fill OutputHistoryModel from FormStatus
737d94d 
Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 20, 2024
@mstv
feat(FormStatusOutputLog): Normalize line endings
946c5aa 
and vertical tabs

Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 20, 2024
@mstv
feat: Add OutputHistoryControl and controllers
b7493d3 
Refs: gitextensions#10307
@gerhardol
Copy link
Member

How to progress this PR?

@mstv
Copy link
Member Author

mstv commented Oct 24, 2024 •
edited
Loading

How to progress this PR?

I am going to merge it after v5.1 (which - by the way - is almost ready for release) as I have got two approvals and no veto.

@gerhardol
Copy link
Member

How to progress this PR?

I am going to merge it after v5.1 (which - by the way - is almost ready for release) as I have got two approvals and no veto.

Next should be 5.1, from master not cherry picking to a 5.0.1 (as the milestone).
Then master could migrate to .net9 and possibly add a dark theme again.
For me, this could be added before 5.1

@pmiossec
Copy link
Member

For me, this could be added before 5.1

I agréé.

mstv added a commit to mstv/gitextensions that referenced this pull request Oct 25, 2024
@mstv
feat: Add OutputHistoryModel
d74f159 
Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 25, 2024
@mstv
feat: Fill OutputHistoryModel from FormStatus
6744d5a 
Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 25, 2024
@mstv
feat(FormStatusOutputLog): Normalize line endings
34eee55 
and vertical tabs

Refs: gitextensions#10307
mstv added a commit to mstv/gitextensions that referenced this pull request Oct 25, 2024
@mstv
feat: Add OutputHistoryControl and controllers
dc037c9 
Refs: gitextensions#10307
@mstv mstv force-pushed the feature/10206_git_output_tab branch from 5e3c795 to dc037c9 Compare October 25, 2024 21:59
@mstv mstv force-pushed the feature/10206_git_output_tab branch from dc037c9 to 55414f0 Compare October 25, 2024 22:15
@mstv mstv merged commit 55414f0 into gitextensions:master Oct 25, 2024
4 checks passed
@mstv mstv deleted the feature/10206_git_output_tab branch October 25, 2024 22:32
@RussKie RussKie added this to the 5.1 milestone Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RussKie RussKie RussKie left review comments

@pmiossec pmiossec pmiossec approved these changes

@gerhardol gerhardol gerhardol approved these changes

Assignees

@mstv mstv

Labels
👓 status: needs review status: ready
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

Keep git output
6 participants
@mstv @gerhardol @PeterMinin @RussKie @vbjay @pmiossec