Add Notes column.

[pmgiant]

I need to see git notes in a separate column of RevisionGridControl. My notes are usually short and often re-generated by scripts.

The only initially visible change is a new menu item: View / Show git notes column.
Checking it adds a new column to the revision grid, between "Message" and "Avatar".
This column displays the first row of a note.
Notes displayed by CommitDataBodyRenderer (Commit tab of the "Commit info" pane) and MessageColumnProvider remain unaffected.

Checking either one of the "Show notes..." menu items enables note loading. Checking only the old menu item "Show notes" also shows notes ref. Checking only the "Show notes column" item leads to displaynig notes text in columns and commit info, but hiding notes "ref".

I`ve simplified the code a bit, now the CommitData and GitRevision classes have separate members for "Body" and "Notes". Parsing/re-formatting logic has been adjusted accordingly, code duplicates were not fixed in order to minimise footprint.
This change probably also fixes some potential bugs, as the notes are no longer part of the content of the message body internally.

~Fixes #11413

Proposed changes

Screenshots

Before

Originally without the column (menu item un-checked).

After

Feature turned on, notes not displayed in MessageColumn, but in the separate one.
Either case, the notes are displayed in the "commit info" html pane, there`s plenty of space there.

Test methodology

Test environment(s)

• GIT
• Windows

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.

[RussKie]

Could you please provide screenshots as well?

[pmiossec]

You have build issue to fix: https://ci.appveyor.com/project/gitextensions/gitextensions/builds/48652155

First, I like the idea. It cool make the git note feature really useful whereas at the moment I never really used it...

But I wonder if it's not a setting I would like to enable per repository. Because some repos will have notes but some other not. And I don't want to have to spend all my time to enable/disable it every times when switching between repos. Except if the size is automatically set to 0 if there is none displayed.

[pmgiant]

Thanks, sure thing, different code styles...

Perhaps it can be done, I`m new to the codebase, so parts of the change were done quickly, the copy-pase way. Perhaps the appSetting global parameter is not the way to go. Maybe someone could point me to the per-repository settings mechanism :) ?

[pmiossec]

Perhaps it can be done, I`m new to the codebase, so parts of the change were done quickly, the copy-pase way. Perhaps the appSetting global parameter is not the way to go. Maybe someone could point me to the per-repository settings mechanism :) ?

Hold on! It's not an order to do so 🤣 I would like to start a discussion on the subject. We will see others point of view...

[vbjay]

But I wonder if it's not a setting I would like to enable per repository. Because some repos will have notes but some other not. And I don't want to have to spend all my time to enable/disable it every times when switching between repos. Except if the size is automatically set to 0 if there is none displayed.

Just like build server config, allow scoped setting. Global, distributed with repo, local repo. That way the user can set a default in global and if a specific repo should have it then setting in distributed/local can override the global setting.

[pmiossec]

But I wonder if it's not a setting I would like to enable per repository. Because some repos will have notes but some other not. And I don't want to have to spend all my time to enable/disable it every times when switching between repos. Except if the size is automatically set to 0 if there is none displayed.

Just like build server config, allow scoped setting. Global, distributed with repo, local repo. That way the user can set a default in global and if a specific repo should have it then setting in distributed/local can override the global setting.

Yes, that's what I was thinking. But that means that no more menuitem in the View menu but more here maybe:

[pmiossec]

@pmgiant to fix the build, run the script update-loc.cmd that you will find at the repository root folder.

[pmgiant]

I have a question - I`ve been working with the debug build of gitExtensions for some time now. Everything is fine, except for one thing - there are numerous quite suspicious assserts failing from time to time. Shall I report them as issues?

[vbjay]

But I wonder if it's not a setting I would like to enable per repository. Because some repos will have notes but some other not. And I don't want to have to spend all my time to enable/disable it every times when switching between repos. Except if the size is automatically set to 0 if there is none displayed.

Just like build server config, allow scoped setting. Global, distributed with repo, local repo. That way the user can set a default in global and if a specific repo should have it then setting in distributed/local can override the global setting.

Yes, that's what I was thinking. But that means that no more menuitem in the View menu but more here maybe:

Menu is possible with a sub menu of which scope.

[gerhardol]

Perhaps it can be done, I`m new to the codebase, so parts of the change were done quickly, the copy-pase way. Perhaps the appSetting global parameter is not the way to go. Maybe someone could point me to the per-repository settings mechanism :) ?

Hold on! It's not an order to do so 🤣 I would like to start a discussion on the subject. We will see others point of view...

I am not convinced that is needed. It is only a column visible or not.
(appsetting can be global/repo etc too)

[gerhardol]

Mostly fine.

[mstv]

just refinement

[mstv]

in addition to the remaining not yet resolved comments

Please avoid unnecessary force-pushes in order to ease incremental reviews. "Fixup!" commits will be squashed right before or on merge.

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 14 days. It will be closed if no further activity occurs.

[pmgiant]

As suggested above by "mstv", I created a new "fixup!..." commit instead of squashing consecutive tiny code refactorings. Now the "block-fixup" check fails.

[pmiossec]

As suggested above by "mstv", I created a new "fixup!..." commit instead of squashing consecutive tiny code refactorings. Now the "block-fixup" check fails.

fixup are intended to be temporary to make review easier and to be squash just before merging the PR (so that the check pass).

[RussKie]

As a general rule in this repo, an author presents the work in a form that the author expects to get merged. Thus we have a rule to not merge fixups. Though, these days we generally squash merge, so fixups isn't such an issue. However, we still prefer the author to clean up the history and commit messages as those will stay around for long time :)

[pmgiant]

So maybe let's wait for a day od two, if someone wants to take a look at these few last (hopefully) polishes.
If no one wants to add any more comments - I'll squash these two commits into a single, concise change.

[mstv]

unfinished refinement

[pmgiant]

@mstv, I don't get the 'exclamation mark' comments above about UIExtensions.cs:64 and FormBrowse.cs:663.

[mstv]

I don't get the 'exclamation mark' comments above about UIExtensions.cs:64 and FormBrowse.cs:663.

Several comments of my first review have not been addressed at all: refer to #11417 (review) (might need to load comments hidden by GH).

[pmgiant]

All items have been adressed there, as far as I can count to 14 and refresh a web page late in the night.. :)

[mstv]

Several comments of my first review have not been addressed at all

All items have been adressed there, as far as I can count to 14 and refresh a web page late in the night.. :)

might need to load comments hidden by GH

#11417 (comment)
#11417 (comment)
#11417 (comment)

[pmgiant]

Weird, some reviews expanded by default were displayed somewhat incomplete and read-only. Their commentable counterparts could be found after some scrolling and expanding.
For now they all look addressed properly :)

[mstv]

have not run

[mstv]

English.xlf needs to be updated or the ampersand for the menu item to be restored - as you like.