Show git-notes

[CyberShadow]

This PR implements basic support for displaying notes created by git notes (or software supporting it, such as magit).

The notes are read from Git's default ref location (refs/notes/commits), and thus it is assumed that the ref is pushed at that location to Gitea.

The implementation here is minimal; if the idea is OK, I can do any necessary changes.

[codecov-io]

Codecov Report

Merging #6984 into master will decrease coverage by <.01%.
The diff coverage is 29.16%.

@@            Coverage Diff             @@
##           master    #6984      +/-   ##
==========================================
- Coverage   41.49%   41.49%   -0.01%     
==========================================
  Files         440      441       +1     
  Lines       59459    59507      +48     
==========================================
+ Hits        24674    24692      +18     
- Misses      31566    31589      +23     
- Partials     3219     3226       +7

Impacted Files	Coverage Δ
routers/repo/commit.go	24.23% <0%> (-0.77%)	⬇️
modules/templates/helper.go	47.35% <0%> (-1.08%)	⬇️
modules/git/notes.go	43.75% <43.75%> (ø)
models/repo_list.go	73.09% <0%> (+1.01%)	⬆️
models/unit.go	67.56% <0%> (+5.4%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5a98a2...dcbef74. Read the comment docs.

[lunny]

Any screenshot? And could you change the text on template to locale_en_US.ini?

[zeripath]

I like the idea of this - however, is there any way to avoid reading the whole thing in memory and or making the page too big to render?

How about test the size of the note - if it's less than say 5Kb (most notes will be) then read it in otherwise provide a link that that the UI can asynchronously read. You can stick that as an API route and then you can test the note reading infrastructure relatively easy.

I think in general we should be more careful around ReadAll and page sizes.

[CyberShadow]

Any screenshot?

Sure:

Although git displays notes directly beneath commit messages, I opted to place the notes outside the commit box in Gitea's UI, because their authorship is not tied to the commit's author/committer. Placing them right by the commit message would make it look like they were written by the same person as the rest of the commit.

And could you change the text on template to locale_en_US.ini?

Done.

I like the idea of this - however, is there any way to avoid reading the whole thing in memory and or making the page too big to render?

Sorry, could you please explain why you think this is necessary?

As far as I know, the typical use of git-notes is editable additions to commit messages. The git-notes manual states this as well, so, there's the implication that notes and commit messages shouldn't be treated too differently. I'm not familiar of a use case for git-notes which would involve placing large amounts of data in the notes objects.

Even though it's possible to create arbitrarily large git notes, it's also possible to create arbitrarily large commit messages. As far as I can see, Gitea will always display the entire commit message, without limiting it, and without providing a view to see it in its entirety. So, it doesn't make sense to me to do this for git notes without also doing it for commit messages, considering that there don't seem to be any pertinent distinctions between the two.

[lunny]

Could you display the author and time of the git note?

[CyberShadow]

I think it's possible, by looking at the last commit in the notes ref history which updated the note's path.

Note that it would be more accurate to say that it would be the person who last edited the note, since if you want to add a note to a commit and a note already exists for that commit, you have no choice but to edit the existing note.

[CyberShadow]

Could you display the author and time of the git note?

Done, how does this look?

[lafriks]

Could this be added in frame together with commit message?

[CyberShadow]

Something like this?

It doesn't look good in the dark theme, will likely need new CSS rules there.

[sapk]

AFAIK, notes are not signed by gpg so maybe it is better to display them under the signature.

[CyberShadow]

You certainly can sign the notes (by making signed commits in the notes ref), but that's a completely distinct signature from the commit that the notes are annotating.

Edit: You're right, git-notes does not sign commits made in the notes ref even when commit signing is otherwise enabled, so this is not a circumstance that we need to worry about.

[sapk]

This PR could be improved by :

• Filtering display of big note that could break ui
• Add tests to validate GetNote func
• Displaying that a note exist (like gpg lock) in commit list and a preview on passing over
• I personally would prefer to display notes like GPG. Attached to the commit block to display that it is linked but under GPG (since itsn't verifed by it)

But this lay down some simple bases that works and could be improved later. And since it is not a main feature those changes are not blocking from my point of view. LGTM

[CyberShadow]

Filtering display of big note that could break ui

I think that if we do this, it should also be done for commit messages.

Add tests to validate GetNote func

Test added :)

Displaying that a note exist (like gpg lock) in commit list and a preview on passing over

Good idea, I may submit a follow-up PR adding this.

I personally would prefer to display notes like GPG. Attached to the commit block to display that it is linked but under GPG (since itsn't verifed by it)

Do you have a mockup of your idea? Would it be essentially as the second screenshot I posted but without the gap between the blocks?

[sapk]

It should also be done for commit messages.

Yes but that is for an other PR. It could be good to do it directly for notes but that isn't a blocking point (at least for me) as I suppose that people using note will understand and it could be fixed later.

Do you have a mockup of your idea?

I have done some testing here : sapk-fork@2f453e7

The main problem is that bottom class style does a lot to the GPG commit formatting. For testing I had to use inline css to counter that. The closest class would be ui attached block header.
I suggest you to not take time on this until other people validate that they prefer this way ;-)

[lafriks]

Yes I was thinking about something like what @sapk did

[CyberShadow]

I like what you did with the header so I adapted that:

I am not sure about gluing the note block to the commit block, for two reasons:

1. I think it looks a bit too clumped-up, it's not obvious to me at a first glance where the information about the commit ends and information about the note begins.
2. Hypothetically, a repository can contain multiple refs storing git notes. These can be due to having been written by different authors, or generated by different tools. It's possible that in the future users of Gitea might want to show notes from more than one ref (e.g. by having a list of refs as a per-repository config); in this case there probably should be one bubble per notes ref containing a note for the commit.

Of course you have the final word :)

[techknowlogick]

@CyberShadow I like what you did in the screenshots, as it will allow showing that notes are GPG signed similar to how commits are (to be done in a future PR of course).

[lafriks]

This looks really nice now

[lafriks]

Those two small suggestions but otherwise looks good

[lafriks]

Thanks for PR ;)