prati0100 / git-gui

Tcl/Tk based UI for Git. I am currently acting as the project's maintainer.
160 stars 87 forks source link

git-gui commit text widget stays fixed in size when expanding the top window horizontally #53

Open dzach opened 3 years ago

dzach commented 3 years ago

Hi This refers to the latest Tcl/Tk git-gui.sh code git version 2.17.1, from https://github.com/git/git. It's not a new thing. It has been present in git-gui since the beginning, I believe.

Problem: When expanding the top level window of the stock git-gui interface, the 'Commit Message' text widget remains fixed in size. Long message lines may be partially hidden, since the same text widget has its -wrap option set to none, and the rest of the now expanded area to the right of the text widget remains empty, as far as I can see, unless there is some other functionality that uses that empty space, which I have not encountered over the years.

Proposed solution

  1. Allow the 'Commit Message' text widget and its containers to expand to the edges of the enclosing frame, when the top window changes size (patch 1).
  2. Change the text widget's -wrap option to word and
  3. Adjust spacing between (non wrapped) lines to improve text appearance (patch 2).

Attached are two trivial patches and three images of the resulting appearance.

Thanks.

git-gui.zip

prati0100 commented 3 years ago

@dzach Thanks for the patches. The current width of the commit message window matches exactly the "Commit message text width" option in the options menu. This is supposed to give a visual cue to the user that they are reaching their desired commit message width limit and they should break the line. I don't think it is wise to break that flow without providing a replacement.

One option that I considered was to introduce line wrapping. The line wrapping in your patches is soft wrapping. It only wraps the line visually. The actual line in memory stays intact. This would mean that the text in the commit message will not be wrapped. I am thinking more about hard line wrapping, something like the ones done by text editors like vim or emacs.

I welcome patches if you are interested in that feature. But please don't send them in a zip file. They are very hard to review and provide feedback on in that format. Look at the README to see how you can send patches.

dzach commented 3 years ago

Thank you for the reply @prati0100

I'm sorry I overlooked the instructions for how to contribute.

prati0100 commented 3 years ago

Thank you for the reply @prati0100

  • You are right, changing the widget's width indeed removes the visual cue of the message width set in the options, and the intent for having that option in the first place is clear. However, the absence of a horizontal scrollbar makes this visual cue work only when the actual line length is less that the set width, since the line shifts to the left as soon as it exceeds the widget's width, thus obscuring its start, and the visual cue is lost.

There is a horizontal scrollbar present at the bottom of the text box. It is only activated when a line actually crosses the text box width. It can serve as a visual cue that one of your lines has crossed the line length. And you can always scroll back to the start for other lines.

  • Providing a soft wrap, but keeping the width fixed, keeps the visual cue in place, while at the same time allowing the first line to be visible in all its length and continue growing for as long as the user wants it, without its start being obscured. In a way, the widget's width option is itself a "soft" option, in that it is not forcing a hard limit to the user but is just hinting it.

Right, keeping a soft wrap does make it a bit easier to read the full message with some lines which are too long. But it has some downsides as well. For example, from your screenshots I can see that the difference between a soft-wrapped line and the next line is very subtle and not every user might notice it, or notice it quickly enough.

  • The reason I believe the first line is important is because some git GUI front-ends, including gitk, only show the first line of the commit message in their graph as a description; a user places a hard break in the first line where he needs this description to end. Forcing automatic hard breaks will remove this option from the user.

Usually, the practice is to make sure your subject line never exceeds the width. But I don't think tools should enforce this unconditionally. They should give the users tools to enforce it if they so wish.

So when I say hard wrap, I mean that it should include an option to not hard wrap a line when a user doesn't want it. We can look at popular text editors and see how they do this. For example, in vim you can set it to automatically hard wrap but when you press J, it joins the wrapped lines and doesn't auto wrap it until you tell it to do it manually.

Just so this response is not just a bunch of vague arguments, I'll summarize what I think. I think the current system is fine, although there is certainly room for improvement. Increasing the text box width is not one of them.

I am open to a soft wrap option but I think it will be difficult to get right. Quickly differentiating between lines with soft and hard breaks is an important thing and I am not convinced Tk's current implementation does a very good job of it. So if you do want to implement soft wraps either figure out a way to very clearly differentiate between the two or make it an option disabled by default.

My most preferred option would be to add hard wraps with the ability to manually make lines larger than the limit if the user wants to do so.

dzach commented 3 years ago

Ok. I'll try to work out some solutions for both soft and hard wrap, as soon as free time perimits.