gitui icon indicating copy to clipboard operation
gitui copied to clipboard

View arbitrarily long multi-line annotations for git tags

Open skyfaller opened this issue 3 years ago • 2 comments

Is your feature request related to a problem? Please describe. Happily, we can view git tag annotations, including annotations with multiple lines. Sadly, if the annotation is long and goes on for many lines, we can't see the whole thing. In my current terminal emulator, gitui only displays the first 23 lines.

This issue will increase in importance if we support creating multiple-line annotations for git tags.

Describe the solution you'd like Adding a scrollbar seems like a good solution to me.

If adding scrollbars to a "popup window" is not desirable, perhaps refactoring the UI so that annotations can be viewed in a multi-panel interface, like the files list, might work better.

Additional context Screenshot of gitui with a long git tag annotation and no way to scroll to read the rest of it

skyfaller avatar Feb 18 '22 21:02 skyfaller

This issue has been automatically marked as stale because it has not had any activity half a year. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 28 '22 04:09 stale[bot]

Are there obvious blockers to using draw_scrollbar as y'all once did for the revlog? https://github.com/extrawurst/gitui/issues/868

skyfaller avatar Sep 28 '22 22:09 skyfaller

Are there obvious blockers to using draw_scrollbar

I don't think so

extrawurst avatar Jan 11 '23 08:01 extrawurst

yeah i think this should be pretty simple to do using the machinery for scrollbars we use in a lot of places already

extrawurst avatar Feb 27 '24 14:02 extrawurst

Hi, I started working on this, i'm adding a new popup with a scrollbar for the tag annotation instead of using MsgPopup. should have a PR up soon :)

MichaelAug avatar Mar 05 '24 00:03 MichaelAug

Why not the exiting one and add the scrollbar?

extrawurst avatar Mar 05 '24 00:03 extrawurst

Because currently the annotation is shown using:

	fn show_annotation(&self) {
		if let Some(tag) = self.selected_tag() {
			if let Some(annotation) = &tag.annotation {
				self.queue.push(InternalEvent::ShowInfoMsg(
					annotation.clone(),
				));
			}
		}
	}

Which calls

			InternalEvent::ShowInfoMsg(msg) => {
				self.msg_popup.show_info(msg.as_str())?;
				flags
					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
			}

I didn't want to add a scrollbar to msg_popup as that is used in other places as well so decided to add a separate popup.

I'm happy to just add a scroll bar to the info popup if that's preferable

MichaelAug avatar Mar 05 '24 00:03 MichaelAug

I want to choose the solution that's best for the project and keeps things consistent. Would you recommend adding a scrollbar to the MsgPopup, creating a new popup for tag annotations, or is there another approach you'd prefer? Thanks for your input!

MichaelAug avatar Mar 05 '24 12:03 MichaelAug

you raised a good point. i had to take a look again. my first hunch was: lets use the new multiline textinput but that one does not support readonly right now and is already a rather complex beast.

So in order to keep things simple you can just add vertical scrolling to MsgPopup (using DetailsComponent maybe for inspiration as it supports multiline scrollable text view already).

Ideally we put that functionality into a separate ScrollableTextView component and reuse it between (at least) MsgPopup and DetailsComponent. Both doing this in one step or in two would be fine to me. WDYT?

extrawurst avatar Mar 05 '24 13:03 extrawurst

@MichaelAug for more instant exchange amongst contributors feel free to join our discord: https://discord.gg/d2p2NS7m

extrawurst avatar Mar 05 '24 13:03 extrawurst

Making a reusable component that is used in MsgPopup and DetailsComponent sounds good. I can make the change in a single PR or split into 2 (make the MsgPopup scrollable in the first PR, then refactor DetailsComponent and MsgPopup to use the generic 'ScrollableTextView' in the second PR), whichever you prefer :)

MichaelAug avatar Mar 05 '24 18:03 MichaelAug

@MichaelAug for more instant exchange amongst contributors feel free to join our discord: https://discord.gg/d2p2NS7m

Thanks i'm already in the Discord :+1:

MichaelAug avatar Mar 05 '24 18:03 MichaelAug

I made a PR for the first part. After this is approved I'll make another PR to refactor MsgPopup and DetailsComponent to use a new ScrollableTextView component

MichaelAug avatar Mar 10 '24 17:03 MichaelAug