Skip to content
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.

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

Fix #6276: "Show messages of annotated tags" truncates to ten lines without notice #9616

Merged
merged 1 commit into from Oct 16, 2021

Conversation

lhiginbotham
Copy link
Contributor

@lhiginbotham lhiginbotham commented Oct 5, 2021

Fixes #6276: "Show messages of annotated tags" truncates to ten lines without notice

Proposed changes

  • use git cat-file -p <tag name> to get full tag message, preventing the previous 10 line limitation

Screenshots

Before

image

After

image

Test methodology

Tested the following scenarios:

  • a non-annotated tag (result: no special treatment, tag name does not show in commit message area)
  • a tag with new line characters only (result: same as non-annotated tag)
  • a tag with a null message (result: name appears, no message)
  • a tag with varying numbers of new lines between lines of text (result: multiple new lines condense into a single new line, message shows)
  • a tag with more than ten lines of text (result: all lines of text appear)

Test environment(s)

  • GIT 2.33.0.windows.1
  • Microsoft Windows NT 10.0.19042.0

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.

Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Do you think you could add an non-UI integration a unit test to GitCommands.Tests (i.e. with ReferenceRepository)?

@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Oct 6, 2021
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long captions are truncated to not overflow the user with information. There has been several changes to limit commit messages in various tooltips for instance.
If number of branches exceeds 10, they are limited in the commit info.

This goes the other way. It could be done as "add ore info" too.
Or just add a comment like: No check for length, also displaying very long annotation messages

GitCommands/Git/GitModule.cs Outdated Show resolved Hide resolved
GitCommands/Git/GitModule.cs Outdated Show resolved Hide resolved
@lhiginbotham
Copy link
Contributor Author

lhiginbotham commented Oct 9, 2021

@mstv, you said:

Long captions are truncated to not overflow the user with information. There has been several changes to limit commit messages in various tooltips for instance. If number of branches exceeds 10, they are limited in the commit info.

This goes the other way. It could be done as "add [m]ore info" too. Or just add a comment like: No check for length, also displaying very long annotation messages

Sorry, but are you meaning that I must add a comment in the code to warn devs that we are allowing for potentially large annotation messages..? Or are you suggesting I expose an option to the user to either display a truncated message (probably with an ellipsis at the end [...]) vs the full annotation message?

Or are you suggesting nothing at all, just making a general observation for everyone's information?

Edit: @gerhardol said this, sorry!!

@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Oct 9, 2021
…o ten lines without notice

	- shows the entire annotated tag message, whereas we were previously cutting it off after 10
	- adds happy-path tests for GetTagMessage
	- TODO: in the future, we should add sad-path tests, which probably will require refactoring of GetTagMessage (cases such as passing in a non-tag reference)
@lhiginbotham
Copy link
Contributor Author

@RussKie I wrote the tests that you have asked for. They helped me catch the fact that I was accidentally appending an extraneous new line character on the end of the GetTagMessage string! (Due to calling AppendLine on all message rather than on all except the last, where Append would have been more appropriate for the last message line).

I left a note saying that we should put in "sad-path" tests to test things like if GetTagMessage is passed a non-tag reference, but doing so would require some refactoring, which might be better left for a separate refactoring issue where we can attempt to generalize GetTagMessage into something like GetObjectMessage (if this is even necessary, seeing as there is already logic somewhere for rendering commit message bodies; at the very least, we can put a return null; if GetTagMessage is passed a non-tag ref).

If you are fine with leaving things as-is, this PR should be good to go!

@lhiginbotham
Copy link
Contributor Author

@RussKie are there any pending items on this PR?

@RussKie
Copy link
Member

RussKie commented Oct 10, 2021

I'd like to get @gerhardol sign off

Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The info is only shown in commit info, not in tooltips what I see, so the change should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Show messages of annotated tags" truncates to ten lines without notice
3 participants