[PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Bert Wesarg]

The commit message widget does not wrap the next and has a configurable
fixed width to avoid creating too wide commit messages. Though this was
only enforced in the GUI. Now we also check the commit message at commit
time for long lines and ask the author for confirmation if it exceeds the
configured line length.

Needs Tcl 8.6 because of `lmap`.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 git-gui.sh     |  4 ++--
 lib/commit.tcl | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 5bc21b8..a491085 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -31,8 +31,8 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA}]
 ##
 ## Tcl/Tk sanity check
 
-if {[catch {package require Tcl 8.4} err]
- || [catch {package require Tk  8.4} err]
+if {[catch {package require Tcl 8.6} err]
+ || [catch {package require Tk  8.6} err]
 } {
 	catch {wm withdraw .}
 	tk_messageBox \
diff --git a/lib/commit.tcl b/lib/commit.tcl
index 83620b7..fa9760b 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -215,6 +215,16 @@ A good commit message has the following format:
 		unlock_index
 		return
 	}
+	if {[tcl::mathfunc::max {*}[lmap x [split $msg "\n"] {string length $x}]] >= $repo_config(gui.commitmsgwidth) \
+	    && [ask_popup "Commit message contains lines longer than $repo_config(gui.commitmsgwidth) characters.
+
+You may change this limit in the options.
+
+Continue to commit?
+"] ne yes} {
+		unlock_index
+		return
+	}
 
 	# -- Build the message file.
 	#
-- 
2.21.0.789.ga095d9d866

[PATCH 2/2] git-gui: add horizontal scrollbar to commit buffer

[Bert Wesarg]

While the commit message widget has a configurable fixed width, it
nevertheless allows to write commit messages which exceed this limit.
Though it does not show this content because there is not scrollbar for
this widget. No it is.

There seems to be a bug in at least up to Tcl/Tk 8.6.8, which does not
update the horizontal scrollbar if one removes the whole content at once.

Suggested-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 git-gui.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index a491085..fa9c0d2 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3363,14 +3363,20 @@ ttext $ui_comm -background white -foreground black \
 	-relief sunken \
 	-width $repo_config(gui.commitmsgwidth) -height 9 -wrap none \
 	-font font_diff \
+	-xscrollcommand {.vpane.lower.commarea.buffer.frame.sbx set} \
 	-yscrollcommand {.vpane.lower.commarea.buffer.frame.sby set}
+${NS}::scrollbar .vpane.lower.commarea.buffer.frame.sbx \
+	-orient horizontal \
+	-command [list $ui_comm xview]
 ${NS}::scrollbar .vpane.lower.commarea.buffer.frame.sby \
+	-orient vertical \
 	-command [list $ui_comm yview]
 
+pack .vpane.lower.commarea.buffer.frame.sbx -side bottom -fill x
 pack .vpane.lower.commarea.buffer.frame.sby -side right -fill y
-pack $ui_comm -side left -fill y
+pack $ui_comm -side left -fill both -expand 1
 pack .vpane.lower.commarea.buffer.header -side top -fill x
-pack .vpane.lower.commarea.buffer.frame -side left -fill y
+pack .vpane.lower.commarea.buffer.frame -side bottom -fill both -expand 1
 pack .vpane.lower.commarea.buffer -side left -fill y
 
 # -- Commit Message Buffer Context Menu
-- 
2.21.0.789.ga095d9d866

Re: [PATCH 2/2] git-gui: add horizontal scrollbar to commit buffer

[Eric Sunshine]

On Wed, Sep 4, 2019 at 4:10 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> While the commit message widget has a configurable fixed width, it
> nevertheless allows to write commit messages which exceed this limit.
> Though it does not show this content because there is not scrollbar for
> this widget. No it is.

"No it is" what?

> Suggested-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>

Re: [PATCH 2/2] git-gui: add horizontal scrollbar to commit buffer

[Bert Wesarg]

On Wed, Sep 4, 2019 at 10:31 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Wed, Sep 4, 2019 at 4:10 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> > While the commit message widget has a configurable fixed width, it
> > nevertheless allows to write commit messages which exceed this limit.
> > Though it does not show this content because there is not scrollbar for
> > this widget. No it is.
>
> "No it is" what?

…there is no scrollbar for this widget. Now there is.

fixed

>
> > Suggested-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>

Re: [PATCH 2/2] git-gui: add horizontal scrollbar to commit buffer

[Pratyush Yadav]

On 04/09/19 10:10PM, Bert Wesarg wrote:
> While the commit message widget has a configurable fixed width, it
> nevertheless allows to write commit messages which exceed this limit.
> Though it does not show this content because there is not scrollbar for

Like we discussed before, it does show the content, you just have to 
scroll by keyboard, and can't scroll by mouse. So maybe reword this?

> this widget. No it is.

I pulled from your GitHub since you seem to have fixed this typo there.

> There seems to be a bug in at least up to Tcl/Tk 8.6.8, which does not
> update the horizontal scrollbar if one removes the whole content at once.
> 
> Suggested-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  git-gui.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index a491085..fa9c0d2 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3363,14 +3363,20 @@ ttext $ui_comm -background white -foreground black \
>  	-relief sunken \
>  	-width $repo_config(gui.commitmsgwidth) -height 9 -wrap none \
>  	-font font_diff \
> +	-xscrollcommand {.vpane.lower.commarea.buffer.frame.sbx set} \
>  	-yscrollcommand {.vpane.lower.commarea.buffer.frame.sby set}
> +${NS}::scrollbar .vpane.lower.commarea.buffer.frame.sbx \
> +	-orient horizontal \
> +	-command [list $ui_comm xview]
>  ${NS}::scrollbar .vpane.lower.commarea.buffer.frame.sby \
> +	-orient vertical \
>  	-command [list $ui_comm yview]
>  
> +pack .vpane.lower.commarea.buffer.frame.sbx -side bottom -fill x
>  pack .vpane.lower.commarea.buffer.frame.sby -side right -fill y
> -pack $ui_comm -side left -fill y
> +pack $ui_comm -side left -fill both -expand 1

Dropping this change does not seem to make a difference. The commit 
message buffer expands on resize even without it. Can you please explain 
why you did this?

>  pack .vpane.lower.commarea.buffer.header -side top -fill x
> -pack .vpane.lower.commarea.buffer.frame -side left -fill y
> +pack .vpane.lower.commarea.buffer.frame -side bottom -fill both -expand 1

I'm not too familiar with pack, but why did you change the side from 
left to bottom? I tested by changing it back to left and didn't notice 
any difference.

>  pack .vpane.lower.commarea.buffer -side left -fill y
>  
>  # -- Commit Message Buffer Context Menu

Other than these couple of minor things, the patch LGTM. Thanks.

-- 
Regards,
Pratyush Yadav

Re: [PATCH 2/2] git-gui: add horizontal scrollbar to commit buffer

[Bert Wesarg]

On Tue, Sep 10, 2019 at 10:28 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>
> On 04/09/19 10:10PM, Bert Wesarg wrote:
> > While the commit message widget has a configurable fixed width, it
> > nevertheless allows to write commit messages which exceed this limit.
> > Though it does not show this content because there is not scrollbar for
>
> Like we discussed before, it does show the content, you just have to
> scroll by keyboard, and can't scroll by mouse. So maybe reword this?
>
> > this widget. No it is.
>
> I pulled from your GitHub since you seem to have fixed this typo there.
>
> > There seems to be a bug in at least up to Tcl/Tk 8.6.8, which does not
> > update the horizontal scrollbar if one removes the whole content at once.
> >
> > Suggested-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> > ---
> >  git-gui.sh | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index a491085..fa9c0d2 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -3363,14 +3363,20 @@ ttext $ui_comm -background white -foreground black \
> >       -relief sunken \
> >       -width $repo_config(gui.commitmsgwidth) -height 9 -wrap none \
> >       -font font_diff \
> > +     -xscrollcommand {.vpane.lower.commarea.buffer.frame.sbx set} \
> >       -yscrollcommand {.vpane.lower.commarea.buffer.frame.sby set}
> > +${NS}::scrollbar .vpane.lower.commarea.buffer.frame.sbx \
> > +     -orient horizontal \
> > +     -command [list $ui_comm xview]
> >  ${NS}::scrollbar .vpane.lower.commarea.buffer.frame.sby \
> > +     -orient vertical \
> >       -command [list $ui_comm yview]
> >
> > +pack .vpane.lower.commarea.buffer.frame.sbx -side bottom -fill x
> >  pack .vpane.lower.commarea.buffer.frame.sby -side right -fill y
> > -pack $ui_comm -side left -fill y
> > +pack $ui_comm -side left -fill both -expand 1
>
> Dropping this change does not seem to make a difference. The commit
> message buffer expands on resize even without it. Can you please explain
> why you did this?
>
> >  pack .vpane.lower.commarea.buffer.header -side top -fill x
> > -pack .vpane.lower.commarea.buffer.frame -side left -fill y
> > +pack .vpane.lower.commarea.buffer.frame -side bottom -fill both -expand 1
>
> I'm not too familiar with pack, but why did you change the side from
> left to bottom? I tested by changing it back to left and didn't notice
> any difference.
>
> >  pack .vpane.lower.commarea.buffer -side left -fill y
> >
> >  # -- Commit Message Buffer Context Menu
>
> Other than these couple of minor things, the patch LGTM. Thanks.

Will re-roll.

Thanks.

>
> --
> Regards,
> Pratyush Yadav

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Eric Sunshine]

On Wed, Sep 4, 2019 at 4:10 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> The commit message widget does not wrap the next and has a configurable

s/next/text/

> fixed width to avoid creating too wide commit messages. Though this was
> only enforced in the GUI. Now we also check the commit message at commit
> time for long lines and ask the author for confirmation if it exceeds the
> configured line length.

Hmm, more confirmation dialogs tend to mean more annoyance for users,
especially considering that the line length limit is a
project-specific _policy_ (so this has the potential to annoy a lot of
people), and also because there often are legitimate reasons for
exceeding the limit (such as pasting in URLs).

As an alternative to a confirmation dialog, how about instead adding a
_warning_ message (perhaps with red text) on the window itself
alongside to the commit message field (below or above it or
something)? Is that something that could be triggered by a text widget
callback?

> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
> diff --git a/lib/commit.tcl b/lib/commit.tcl
> @@ -215,6 +215,16 @@ A good commit message has the following format:
> +       if {[tcl::mathfunc::max {*}[lmap x [split $msg "\n"] {string length $x}]] >= $repo_config(gui.commitmsgwidth) \

Does this take TABs into account?

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Bert Wesarg]

On Wed, Sep 4, 2019 at 10:30 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Wed, Sep 4, 2019 at 4:10 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> > The commit message widget does not wrap the next and has a configurable
>
> s/next/text/

fixed

>
> > fixed width to avoid creating too wide commit messages. Though this was
> > only enforced in the GUI. Now we also check the commit message at commit
> > time for long lines and ask the author for confirmation if it exceeds the
> > configured line length.
>
> Hmm, more confirmation dialogs tend to mean more annoyance for users,
> especially considering that the line length limit is a
> project-specific _policy_ (so this has the potential to annoy a lot of
> people), and also because there often are legitimate reasons for
> exceeding the limit (such as pasting in URLs).

these people did not saw the entered text anyway. they would have
needed to change the option (default to 75 characters) to see what
they have typed. which could have been garbage to begin with.

>
> As an alternative to a confirmation dialog, how about instead adding a
> _warning_ message (perhaps with red text) on the window itself
> alongside to the commit message field (below or above it or
> something)? Is that something that could be triggered by a text widget
> callback?

How about a horizontal scrollbar? This indicates pretty conveniently
and in a standard visual way, that there is more text to the side ;-)


>
> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> > ---
> > diff --git a/lib/commit.tcl b/lib/commit.tcl
> > @@ -215,6 +215,16 @@ A good commit message has the following format:
> > +       if {[tcl::mathfunc::max {*}[lmap x [split $msg "\n"] {string length $x}]] >= $repo_config(gui.commitmsgwidth) \
>
> Does this take TABs into account?

probably not. Does git expands tabs if it wrap lines (git shortlog's -w option)?

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Pratyush Yadav]

On 04/09/19 10:43PM, Bert Wesarg wrote:
[snip]
> > > fixed width to avoid creating too wide commit messages. Though 
> > > this was
> > > only enforced in the GUI. Now we also check the commit message at commit
> > > time for long lines and ask the author for confirmation if it exceeds the
> > > configured line length.
> >
> > Hmm, more confirmation dialogs tend to mean more annoyance for users,
> > especially considering that the line length limit is a
> > project-specific _policy_ (so this has the potential to annoy a lot of
> > people), and also because there often are legitimate reasons for
> > exceeding the limit (such as pasting in URLs).
> 
> these people did not saw the entered text anyway. they would have
> needed to change the option (default to 75 characters) to see what
> they have typed. which could have been garbage to begin with.

Not true in my experience. Sure, no scrollbar appears, but the view of 
the textbox still scrolls when I type in a long line. I can go back to 
seeing the start of the line by moving with the arrow keys.

The point being, you _can_ see what you typed. Do you not get this 
behaviour in Windows (or whatever platform you use)? Maybe that is a 
Tcl/Tk problem?

> > As an alternative to a confirmation dialog, how about instead adding 
> > a
> > _warning_ message (perhaps with red text) on the window itself
> > alongside to the commit message field (below or above it or
> > something)? Is that something that could be triggered by a text widget
> > callback?

I'll chime in with what I think would be a great solution: auto word 
wrapping. This way, the users can set the text width, and not have to 
worry about manually formatting it. Long "words" like URLs would still 
get to be on one line, and we avoid showing annoying dialogs.

-- 
Regards,
Pratyush Yadav

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Birger Skogeng Pedersen]

On Thu, Sep 5, 2019 at 12:48 AM Pratyush Yadav <me@yadavpratyush.com> wrote:
> I'll chime in with what I think would be a great solution: auto word
> wrapping. This way, the users can set the text width, and not have to
> worry about manually formatting it. Long "words" like URLs would still
> get to be on one line, and we avoid showing annoying dialogs.

+1. IMO git-gui really needs automatic word wrapping for the commit messages.

Birger

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Bert Wesarg]

On Thu, Sep 5, 2019 at 8:50 AM Birger Skogeng Pedersen
<birger.sp@gmail.com> wrote:
>
> On Thu, Sep 5, 2019 at 12:48 AM Pratyush Yadav <me@yadavpratyush.com> wrote:
> > I'll chime in with what I think would be a great solution: auto word
> > wrapping. This way, the users can set the text width, and not have to
> > worry about manually formatting it. Long "words" like URLs would still
> > get to be on one line, and we avoid showing annoying dialogs.
>
> +1. IMO git-gui really needs automatic word wrapping for the commit messages.

Please exclude the first line, i.e., the subject. This should not be
wrapped at all.

I also imagine, that we should make it configurable, I'm unsure if I
would use this feature for my self.

Bert

>
> Birger

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Birger Skogeng Pedersen]

Hi Bert,


We should probably distinguish between what is wrapped in git-gui
(i.e. purely visual), and what is actually wrapped in the commit
message.
I believe the former is referred to as "soft wrap", while the latter
is "hard wrap".


On Thu, Sep 5, 2019 at 7:46 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Please exclude the first line, i.e., the subject. This should not be
> wrapped at all.

I think all lines should be soft wrapped. Scrolling sideways is just
not something I'd want to do in the gui.

How about we soft wrap all lines (in gui). But when the commit is
created, the actual hard wrap (newline characters) happens only on
lines other than the first one?

But then again, the user might get frustrated when the resulting
commit message looks different than what it appeared in git-gui.

Honestly I'd prefer just wrap the first line as well. If the user gets
frustrated that the first line gets wrapped there are two options:
- Refrain from writing such a long line
- Disable word wrapping (it should be configurable, like you said)

Thoughts?



Birger

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Philip Oakley]

Hi Birger,

On 06/09/2019 15:08, Birger Skogeng Pedersen wrote:
> Hi Bert,
>
>
> We should probably distinguish between what is wrapped in git-gui
> (i.e. purely visual), and what is actually wrapped in the commit
> message.
> I believe the former is referred to as "soft wrap", while the latter
> is "hard wrap".
>
>
> On Thu, Sep 5, 2019 at 7:46 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> Please exclude the first line, i.e., the subject. This should not be
>> wrapped at all.
> I think all lines should be soft wrapped. Scrolling sideways is just
> not something I'd want to do in the gui.
>
> How about we soft wrap all lines (in gui). But when the commit is
> created, the actual hard wrap (newline characters) happens only on
> lines other than the first one?

Not sure if I parsed this correctly, but I'd want a WYSIWYG approach 
that if we wrap on the display it will be wrapped (newline char) in the 
commit. It sounded as if you were proposing a soft wrap visually, but 
not doing the same for the commit.

Personally, I've had both feelings with the gui. I like that the 'hard' 
visual char limit is there that encourages me to wrap my messages.
But at the same time if I'm typing on a flow then it's annoying that 
there wasn't any auto wrap.

The other problem is if one is amending a commit and I need to add a few 
word mid paragraph, the manual re-flowing and manual wrapping can be 
annoying.

I suspect there is a moderately happy medium between the two, perhaps 
with an autowrap key (per paragraph) being available.

I also had it in my head that some parts of Git do allow more than a 
single line headers, or at least can cope with a run-on second line 
before the blank line that flags the start of the message proper. (I may 
be wrong...)
> But then again, the user might get frustrated when the resulting
> commit message looks different than what it appeared in git-gui.
>
> Honestly I'd prefer just wrap the first line as well. If the user gets
> frustrated that the first line gets wrapped there are two options:
> - Refrain from writing such a long line
> - Disable word wrapping (it should be configurable, like you said)
Configurable wrapping point - yes, would be nice (a feeling of control, 
that I'd probably never change ;-).
>
> Thoughts?
>
>
>
> Birger
Philip

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[David Aguilar]

On Fri, Sep 06, 2019 at 09:07:15PM +0100, Philip Oakley wrote:
> Hi Birger,
> 
> On 06/09/2019 15:08, Birger Skogeng Pedersen wrote:
> > Hi Bert,
> > 
> > 
> > We should probably distinguish between what is wrapped in git-gui
> > (i.e. purely visual), and what is actually wrapped in the commit
> > message.
> > I believe the former is referred to as "soft wrap", while the latter
> > is "hard wrap".
> > 
> > 
> > On Thu, Sep 5, 2019 at 7:46 PM Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> > > Please exclude the first line, i.e., the subject. This should not be
> > > wrapped at all.
> > I think all lines should be soft wrapped. Scrolling sideways is just
> > not something I'd want to do in the gui.
> > 
> > How about we soft wrap all lines (in gui). But when the commit is
> > created, the actual hard wrap (newline characters) happens only on
> > lines other than the first one?
> 
> Not sure if I parsed this correctly, but I'd want a WYSIWYG approach that if
> we wrap on the display it will be wrapped (newline char) in the commit. It
> sounded as if you were proposing a soft wrap visually, but not doing the
> same for the commit.
> 
> Personally, I've had both feelings with the gui. I like that the 'hard'
> visual char limit is there that encourages me to wrap my messages.
> But at the same time if I'm typing on a flow then it's annoying that there
> wasn't any auto wrap.
> 
> The other problem is if one is amending a commit and I need to add a few
> word mid paragraph, the manual re-flowing and manual wrapping can be
> annoying.
> 
> I suspect there is a moderately happy medium between the two, perhaps with
> an autowrap key (per paragraph) being available.
> 
> I also had it in my head that some parts of Git do allow more than a single
> line headers, or at least can cope with a run-on second line before the
> blank line that flags the start of the message proper. (I may be wrong...)


Correct, you're thinking about the subject line -- it has that
multi-line behavior.

In git-cola we use a separate LineEdit for the "Commit summary" line,
which has improved the commit messages users write and is good because
the user doesn't need to worry about the "blank line between subject and
body" Git commit message convention.

When the user presses "enter" in the subject line it jumps to the
"Extended description" TextEdit, so it still "feels" like a single
widget.  Likewise, up-arrow from the first line of the Extended
description also jumps to the summary LineEdit.  We also have a "Ctrl-L"
global hotkey to jump back to the subject line (same hotkey as Web
browsers for the "hot" action of focusing the URL).

Using two widgets is a bigger rework for git-gui, but should be
considered for future enhancement.

We also deal with the "Amend a commit" issue through the word-wrap
approach.  It's actually a paragraph reflow (not a line-by-line wrap) so
adding words is fine and easy and does the natural expected thing --
when amending it'll still feel like a long flow line and when committed
it'll wrap with newlines at the configured column.


> > But then again, the user might get frustrated when the resulting
> > commit message looks different than what it appeared in git-gui.
> > 
> > Honestly I'd prefer just wrap the first line as well. If the user gets
> > frustrated that the first line gets wrapped there are two options:
> > - Refrain from writing such a long line
> > - Disable word wrapping (it should be configurable, like you said)
> Configurable wrapping point - yes, would be nice (a feeling of control, that
> I'd probably never change ;-).
> > 
> > Thoughts?


Those are the same controls git-cola has.
- Commit message word-wrap can be turned on/off.  Defaults to on.
- The line length is configurable.  Defaults to 72.

Regarding, "the user might get frusted when the resulting commit message
looks different" -- I don't think that's really a concern in practice
but that might be we have a dedicated LineEdit for the subject line.

For git-gui, it seems fine since Git already has conventions about how
the subject line gets wrapped, so tools will still see the wrapped
subject line as a logical "single line".

The "Extended description" commit message editor is WYSWIYG, but if the
widget is smaller than the configured value then we still wrap the
committed message using the configured value, which is what the user
would likely expect even though the visual wrapping is smaller.

We also special-case trailers like "Signed-off-by:" and other common
trailers since user names can get long, and users sometimes use things
like "See-also:" and paste a long URL that we don't want to wrap.

Lastly, we have a convenient session-only checkbox to temporarily disable
wrapping for a commit that does not persist across restarts.  The idea
is that sometimes you might use the GUI for a one-off commit where you
want to disable the wrapping for whatever reason, but don't want to
change your configuration.
-- 
David

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Pratyush Yadav]

On 10/09/19 01:04AM, David Aguilar wrote:
> On Fri, Sep 06, 2019 at 09:07:15PM +0100, Philip Oakley wrote:
> > 
> > Not sure if I parsed this correctly, but I'd want a WYSIWYG approach that if
> > we wrap on the display it will be wrapped (newline char) in the commit. It
> > sounded as if you were proposing a soft wrap visually, but not doing the
> > same for the commit.
> > 
> > Personally, I've had both feelings with the gui. I like that the 'hard'
> > visual char limit is there that encourages me to wrap my messages.
> > But at the same time if I'm typing on a flow then it's annoying that there
> > wasn't any auto wrap.
> > 
> > The other problem is if one is amending a commit and I need to add a few
> > word mid paragraph, the manual re-flowing and manual wrapping can be
> > annoying.
> > 
> > I suspect there is a moderately happy medium between the two, perhaps with
> > an autowrap key (per paragraph) being available.
> > 
> > I also had it in my head that some parts of Git do allow more than a single
> > line headers, or at least can cope with a run-on second line before the
> > blank line that flags the start of the message proper. (I may be wrong...)
> 
> 
> Correct, you're thinking about the subject line -- it has that
> multi-line behavior.
> 
> In git-cola we use a separate LineEdit for the "Commit summary" line,
> which has improved the commit messages users write and is good because
> the user doesn't need to worry about the "blank line between subject and
> body" Git commit message convention.
> 
> When the user presses "enter" in the subject line it jumps to the
> "Extended description" TextEdit, so it still "feels" like a single
> widget.  Likewise, up-arrow from the first line of the Extended
> description also jumps to the summary LineEdit.  We also have a "Ctrl-L"
> global hotkey to jump back to the subject line (same hotkey as Web
> browsers for the "hot" action of focusing the URL).

I'm not sure how good this idea is. I feel like we should keep it simple 
and trust the user to know what they are doing. Two textboxes seems like 
a lot of clutter for not that much benefit. I've not seen many people 
complain about hitting the extra "enter". Plus, we get to keep our 
message buffer the same as when a user would use a plain text editor.

One idea I have for dealing with users wanting a longer subject line is 
to do something like git-format-patch's subject line is formatted (this 
could be an mbox thing, and not send-email specific). When the subject 
is too long, the subject is broken in multiple lines, but the first 
character on the next line is a space, so it signifies a continued 
subject.

Something like:
Subject: foo bar baz
 abcd  

Which gets converted to "foo bar baz abcd" in the commit message.

> 
> Using two widgets is a bigger rework for git-gui, but should be
> considered for future enhancement.
> 
> We also deal with the "Amend a commit" issue through the word-wrap
> approach.  It's actually a paragraph reflow (not a line-by-line wrap) so
> adding words is fine and easy and does the natural expected thing --
> when amending it'll still feel like a long flow line and when committed
> it'll wrap with newlines at the configured column.

Yes, a paragraph reflow is what I had in mind. Without it editing 
already written lines is a pain.

> > > But then again, the user might get frustrated when the resulting
> > > commit message looks different than what it appeared in git-gui.
> > > 
> > > Honestly I'd prefer just wrap the first line as well. If the user gets
> > > frustrated that the first line gets wrapped there are two options:
> > > - Refrain from writing such a long line
> > > - Disable word wrapping (it should be configurable, like you said)
> > Configurable wrapping point - yes, would be nice (a feeling of control, that
> > I'd probably never change ;-).
> > > 
> > > Thoughts?
> 
> 
> Those are the same controls git-cola has.
> - Commit message word-wrap can be turned on/off.  Defaults to on.
> - The line length is configurable.  Defaults to 72.
> 
> Regarding, "the user might get frusted when the resulting commit message
> looks different" -- I don't think that's really a concern in practice
> but that might be we have a dedicated LineEdit for the subject line.

The aim is to have the commit message be exactly what the user sees. But 
I'm not sure how feasible that is considering the tools/APIs we have to 
work with.
 
> For git-gui, it seems fine since Git already has conventions about how
> the subject line gets wrapped, so tools will still see the wrapped
> subject line as a logical "single line".
> 
> The "Extended description" commit message editor is WYSWIYG, but if the
> widget is smaller than the configured value then we still wrap the
> committed message using the configured value, which is what the user
> would likely expect even though the visual wrapping is smaller.
> 
> We also special-case trailers like "Signed-off-by:" and other common
> trailers since user names can get long, and users sometimes use things
> like "See-also:" and paste a long URL that we don't want to wrap.

Nice catch! Didn't think of that.

> Lastly, we have a convenient session-only checkbox to temporarily 
> disable
> wrapping for a commit that does not persist across restarts.  The idea
> is that sometimes you might use the GUI for a one-off commit where you
> want to disable the wrapping for whatever reason, but don't want to
> change your configuration.

Sounds like a good idea, but I wonder how we can fit it into git-gui's 
current UI layout.

-- 
Regards,
Pratyush Yadav

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Johannes Sixt]

Am 04.09.19 um 22:43 schrieb Bert Wesarg:
> these people did not saw the entered text anyway. they would have
> needed to change the option (default to 75 characters) to see what
> they have typed. which could have been garbage to begin with.

Huh? When I type overly long line, all text scrolls out of view on the
left side. So I definitely _can_ see the long text.

> How about a horizontal scrollbar? This indicates pretty conveniently
> and in a standard visual way, that there is more text to the side ;-)

The scrollbar is an option, of course, but I dislike somewhat that it
takes away screen space if it is permanently visible.

-- Hannes

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Bert Wesarg]

On Thu, Sep 5, 2019 at 8:22 AM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Am 04.09.19 um 22:43 schrieb Bert Wesarg:
> > these people did not saw the entered text anyway. they would have
> > needed to change the option (default to 75 characters) to see what
> > they have typed. which could have been garbage to begin with.
>
> Huh? When I type overly long line, all text scrolls out of view on the
> left side. So I definitely _can_ see the long text.

wrong memory bank, sorry. Though imagine you paste in some multi line
text, and one line is too long but the next not. In this case you wont
see the too long part anymore.

>
> > How about a horizontal scrollbar? This indicates pretty conveniently
> > and in a standard visual way, that there is more text to the side ;-)
>
> The scrollbar is an option, of course, but I dislike somewhat that it
> takes away screen space if it is permanently visible.

I already argued against an auto-hide scrollbar, which was also acked by ohers:

On 02.09.19 19:58, Bert Wesarg wrote:
> On Mon, Sep 2, 2019 at 7:35 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> True, and that reasoning would justify hiding scrollbar when not
>> necessary to gain vertical space.  Can we arrange the scrollbar to
>> appear only when needed?
>
> up to now, git-gui does not hide any scrollbars, if they are not
> needed. IMHO, I would keep it that way, as I don't like the flicker
> when it appears and disappears. Imagine you are typing in the bottom
> line and than you typed too much. The scrollbar appears, your input
> line jumps one up line (or is hidden behind the scrollbar), than you
> remove the too long line, the scrollbar disappears again and your
> input line jumps down again.

>
> -- Hannes

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Johannes Sixt]

Am 04.09.19 um 22:10 schrieb Bert Wesarg:
> The commit message widget does not wrap the next and has a configurable
> fixed width to avoid creating too wide commit messages. Though this was
> only enforced in the GUI. Now we also check the commit message at commit
> time for long lines and ask the author for confirmation if it exceeds the
> configured line length.
> 
> Needs Tcl 8.6 because of `lmap`.
> 
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  git-gui.sh     |  4 ++--
>  lib/commit.tcl | 10 ++++++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 5bc21b8..a491085 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -31,8 +31,8 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA}]
>  ##
>  ## Tcl/Tk sanity check
>  
> -if {[catch {package require Tcl 8.4} err]
> - || [catch {package require Tk  8.4} err]
> +if {[catch {package require Tcl 8.6} err]
> + || [catch {package require Tk  8.6} err]
>  } {
>  	catch {wm withdraw .}
>  	tk_messageBox \
> diff --git a/lib/commit.tcl b/lib/commit.tcl
> index 83620b7..fa9760b 100644
> --- a/lib/commit.tcl
> +++ b/lib/commit.tcl
> @@ -215,6 +215,16 @@ A good commit message has the following format:
>  		unlock_index
>  		return
>  	}
> +	if {[tcl::mathfunc::max {*}[lmap x [split $msg "\n"] {string length $x}]] >= $repo_config(gui.commitmsgwidth) \

This has an off-by-one error: When I fill the edit box to the limit, but
not beyond it, I get the warning popup. I guess this should be '>', not
'>='.

> +	    && [ask_popup "Commit message contains lines longer than $repo_config(gui.commitmsgwidth) characters.
> +
> +You may change this limit in the options.
> +
> +Continue to commit?
> +"] ne yes} {
> +		unlock_index
> +		return
> +	}
>  
>  	# -- Build the message file.
>  	#
> 

-- Hannes

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Bert Wesarg]

On Tue, Sep 10, 2019 at 6:50 PM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Am 04.09.19 um 22:10 schrieb Bert Wesarg:
> > The commit message widget does not wrap the next and has a configurable
> > fixed width to avoid creating too wide commit messages. Though this was
> > only enforced in the GUI. Now we also check the commit message at commit
> > time for long lines and ask the author for confirmation if it exceeds the
> > configured line length.
> >
> > Needs Tcl 8.6 because of `lmap`.
> >
> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> > ---
> >  git-gui.sh     |  4 ++--
> >  lib/commit.tcl | 10 ++++++++++
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index 5bc21b8..a491085 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -31,8 +31,8 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA}]
> >  ##
> >  ## Tcl/Tk sanity check
> >
> > -if {[catch {package require Tcl 8.4} err]
> > - || [catch {package require Tk  8.4} err]
> > +if {[catch {package require Tcl 8.6} err]
> > + || [catch {package require Tk  8.6} err]
> >  } {
> >       catch {wm withdraw .}
> >       tk_messageBox \
> > diff --git a/lib/commit.tcl b/lib/commit.tcl
> > index 83620b7..fa9760b 100644
> > --- a/lib/commit.tcl
> > +++ b/lib/commit.tcl
> > @@ -215,6 +215,16 @@ A good commit message has the following format:
> >               unlock_index
> >               return
> >       }
> > +     if {[tcl::mathfunc::max {*}[lmap x [split $msg "\n"] {string length $x}]] >= $repo_config(gui.commitmsgwidth) \
>
> This has an off-by-one error: When I fill the edit box to the limit, but
> not beyond it, I get the warning popup. I guess this should be '>', not
> '>='.

Thanks. As Pratyush wont pull this one, I separated it and keep it in
a branch in my fork:

https://github.com/bertwesarg/git-gui/tree/bw/warn-wide-commit-message

Bert

>
> > +         && [ask_popup "Commit message contains lines longer than $repo_config(gui.commitmsgwidth) characters.
> > +
> > +You may change this limit in the options.
> > +
> > +Continue to commit?
> > +"] ne yes} {
> > +             unlock_index
> > +             return
> > +     }
> >
> >       # -- Build the message file.
> >       #
> >
>
> -- Hannes

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Pratyush Yadav]

On 04/09/19 10:10PM, Bert Wesarg wrote:
> The commit message widget does not wrap the next and has a configurable
> fixed width to avoid creating too wide commit messages. Though this was
> only enforced in the GUI. Now we also check the commit message at commit
> time for long lines and ask the author for confirmation if it exceeds the
> configured line length.

Thanks for the patch, but I'm not a big fan of this. I agree with Eric 
that more dialogs are annoying. Even as a temporary change until we have 
word wrap support, I don't like it. I hope you don't mind if I only take 
the scrollbar change and drop this.

-- 
Regards,
Pratyush Yadav

Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit

[Bert Wesarg]

On Tue, Sep 10, 2019 at 10:35 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>
> On 04/09/19 10:10PM, Bert Wesarg wrote:
> > The commit message widget does not wrap the next and has a configurable
> > fixed width to avoid creating too wide commit messages. Though this was
> > only enforced in the GUI. Now we also check the commit message at commit
> > time for long lines and ask the author for confirmation if it exceeds the
> > configured line length.
>
> Thanks for the patch, but I'm not a big fan of this. I agree with Eric
> that more dialogs are annoying. Even as a temporary change until we have
> word wrap support, I don't like it. I hope you don't mind if I only take
> the scrollbar change and drop this.

No problem, will keep this patch as a branch. Just wanted to be pro-active.

Bert


>
> --
> Regards,
> Pratyush Yadav