Re: [PATCH v3 4/4] git-gui: allow undoing last revert
On Mon, Oct 21, 2019 at 9:04 PM Pratyush Yadav wrote: > > On 21/10/19 11:16AM, Bert Wesarg wrote: > > Dear Pratyush, > > > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > > the stage-list. But I think it should be disabled, like the 'Revert > > Hunk' and 'Revert Line' menu entry. > > I'm not sure what you mean. There is no "Revert Last Hunk" menu entry (I > assume you are talking about the context menu in the diff view that we > open by right clicking). > > My guess is that you mean the "Undo Last Revert" option. And you want it > disabled if the diff shown is of a staged file, correct? > > I'm not sure if disabling it would be a good idea. > > Say I revert a hunk or line while the file is not staged, and stage the > rest of the file. This would mean that file is no longer in the > "Unstaged Changes" widget. Now if I choose the file from the "Staged > Changes" widget, I get the option to undo the last revert. If I hit > that, it will put whatever I reverted in the "Unstaged Changes" widget. > So, now part of the file that was reverted is in "Unstaged Changes", and > the rest in "Unstaged Changes". > Sorry for this confusion. > IIUC, this is what you think should not happen, correct? What's wrong > with allowing the user to undo reverts from anywhere? The 'Undo' changes the worktree not the staged content,. > > The way I see it, it doesn't really matter what file is selected or > whether it is staged or not, the action of the undo remains the same, so > it makes sense to me to allow it from anywhere. It would make sense to undo the revert on the staged content, if there are no more changes to this file in the worktree. I.e., you wont be able to apply the 'undo' anymore to the worktree file if it is not listed anymore. Though even that case should be able to implement. Though the undo is currently not bound to a specific path. This may be the cause of our different view. I though the undo is bound to the path it was recorded, thus also would only be available when showing a diff on this path again. Therefore I think, having the 'Undo Last Revert' in the context menu may not be the best place to begin with, or at least indicate that this 'undo' operation does not necceseritly operate on the file currently shown. Bertt > > > Can you confirm this? > > > > On Wed, Aug 28, 2019 at 11:57 PM Pratyush Yadav > > wrote: > > > > > > Accidental clicks on the revert hunk/lines buttons can cause loss of > > > work, and can be frustrating. So, allow undoing the last revert. > > > > > > Right now, a stack or deque are not being used for the sake of > > > simplicity, so only one undo is possible. Any reverts before the > > > previous one are lost. > > > > > > Signed-off-by: Pratyush Yadav > > -- > Regards, > Pratyush Yadav
Re: [PATCH v3 4/4] git-gui: allow undoing last revert
On Mon, Oct 21, 2019 at 9:35 PM Johannes Sixt wrote: > > Am 21.10.19 um 11:16 schrieb Bert Wesarg: > > Dear Pratyush, > > > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > > the stage-list. But I think it should be disabled, like the 'Revert > > Hunk' and 'Revert Line' menu entry. > > > > Can you confirm this? > > Technically, it need not be disabled because the hunk being reverted > does not depend on the contents of any of diffs that can be shown. > > The entry should be disabled if reverting the stored hunk fails. But to > know that, it would have to be tried: the file could have been edited > since the hunk was generated so that the reversal of the hunk fails. But the "Undo" changes the worktree not the stage, sure it indirectly also changes the view of the staged content, but that is only a secondary effect. As I only can revert in the worktree list, I think we should be consistent and also only allow to undo the revert in the worktree list. And I think it is independent of 'does the undo apply at all' question. Bert > > Not sure what to do, though. > > -- Hannes
[PATCH] t4014: make output-directory tests self-contained
As noted by Gábor in [1], the new tests in edefc31873 ("format-patch:
create leading components of output directory", 2019-10-11) cannot be
run independently. Fix this.
[1] https://public-inbox.org/git/20191011144650.gm29...@szeder.dev/
Signed-off-by: Bert Wesarg
---
Cc: Denton Liu
Cc: Junio C Hamano
Cc: SZEDER Gábor
---
t/t4014-format-patch.sh | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9facc3a79e..b8969998b5 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1615,17 +1615,20 @@ test_expect_success 'format-patch -o with no leading
directories' '
'
test_expect_success 'format-patch -o with leading existing directories' '
- git format-patch -o patches/side master..side &&
+ rm -rf existing-dir &&
+ mkdir existing-dir &&
+ git format-patch -o existing-dir/patches master..side &&
count=$(git rev-list --count master..side) &&
- ls patches/side >list &&
+ ls existing-dir/patches >list &&
test_line_count = $count list
'
test_expect_success 'format-patch -o with leading non-existing directories' '
- rm -fr patches &&
- git format-patch -o patches/side master..side &&
+ rm -rf non-existing-dir &&
+ git format-patch -o non-existing-dir/patches master..side &&
count=$(git rev-list --count master..side) &&
- ls patches/side >list &&
+ test_path_is_dir non-existing-dir &&
+ ls non-existing-dir/patches >list &&
test_line_count = $count list
'
--
2.23.0.13.g28bc381d7c
Re: [PATCH v5 1/2] format-patch: create leading components of output directory
Please ignore this. Will rebase on 2.24-rc0 and will only include the
test changes.
Bert
On Mon, Oct 21, 2019 at 12:25 PM Bert Wesarg wrote:
>
> 'git format-patch -o ' did an equivalent of 'mkdir '
> not 'mkdir -p ', which is being corrected.
>
> Avoid the usage of 'adjust_shared_perm' on the leading directories which
> may have security implications. Achieved by temporarily disabling of
> 'config.sharedRepository' like 'git init' does.
>
> Signed-off-by: Bert Wesarg
>
> ---
> Changes in v2:
> * squashed and base new tests on 'dl/format-patch-doc-test-cleanup'
>
> Changes in v3:
> * avoid applying adjust_shared_perm
>
> Changes in v4:
> * based on dl/format-patch-doc-test-cleanup and adopt it
>
> Changes in v5:
> * make tests self-contained
>
> Cc: Denton Liu
> Cc: Junio C Hamano
> Cc: SZEDER Gábor
> ---
> Documentation/config/format.txt| 2 +-
> Documentation/git-format-patch.txt | 3 ++-
> builtin/log.c | 16
> t/t4014-format-patch.sh| 26 ++
> 4 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
> index cb629fa769..40cad9278f 100644
> --- a/Documentation/config/format.txt
> +++ b/Documentation/config/format.txt
> @@ -81,7 +81,7 @@ format.coverLetter::
>
> format.outputDirectory::
> Set a custom directory to store the resulting files instead of the
> - current working directory.
> + current working directory. All directory components will be created.
>
> format.useAutoBase::
> A boolean value which lets you enable the `--base=auto` option of
> diff --git a/Documentation/git-format-patch.txt
> b/Documentation/git-format-patch.txt
> index 0ac56f4b70..2035d4d5d5 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -66,7 +66,8 @@ they are created in the current working directory. The
> default path
> can be set with the `format.outputDirectory` configuration option.
> The `-o` option takes precedence over `format.outputDirectory`.
> To store patches in the current working directory even when
> -`format.outputDirectory` points elsewhere, use `-o .`.
> +`format.outputDirectory` points elsewhere, use `-o .`. All directory
> +components will be created.
>
> By default, the subject of a single patch is "[PATCH] " followed by
> the concatenation of lines from the commit message up to the first blank
> diff --git a/builtin/log.c b/builtin/log.c
> index 44b10b3415..8d08632858 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1765,10 +1765,26 @@ int cmd_format_patch(int argc, const char **argv,
> const char *prefix)
> setup_pager();
>
> if (output_directory) {
> + int saved;
> if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
> rev.diffopt.use_color = GIT_COLOR_NEVER;
> if (use_stdout)
> die(_("standard output, or directory, which one?"));
> + /*
> +* We consider as 'outside of gitdir', therefore
> avoid
> +* applying adjust_shared_perm in s-c-l-d.
> +*/
> + saved = get_shared_repository();
> + set_shared_repository(0);
> + switch
> (safe_create_leading_directories_const(output_directory)) {
> + case SCLD_OK:
> + case SCLD_EXISTS:
> + break;
> + default:
> + die(_("could not create leading directories "
> + "of '%s'"), output_directory);
> + }
> + set_shared_repository(saved);
> if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
> die_errno(_("could not create directory '%s'"),
> output_directory);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 72b09896cf..3aab25da76 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1606,6 +1606,32 @@ test_expect_success 'From line has expected format' '
> test_cmp from filtered
> '
>
> +test_expect_success 'format-patch -o with no leading directories' '
> + rm -fr patches &&
> + git format-patch -o patches master..side &&
> + count=$(git rev-list --count master..side) &&
&g
[PATCH v5 2/2] format-patch: configure a command to generate the output directory name
The 'format.outputDirectory' configuration is only able to store constant
directory names. Though some may use
$ git format-patch -o $(createdir) …
to name the directory dynamically. Provide a new configuration to be able
to store such a command too.
Signed-off-by: Bert Wesarg
---
Changes in v2:
* rephrase motivation
Changes in v3:
* remove RFC
Changes in v4:
* based on dl/format-patch-doc-test-cleanup and adopt it
Changes in v5:
* none
Cc: Alexander Kuleshov
Cc: Eric Sunshine
Cc: Denton Liu
Cc: Junio C Hamano
---
Documentation/config/format.txt| 5 +
Documentation/git-format-patch.txt | 6 +-
builtin/log.c | 24 +++-
t/t4014-format-patch.sh| 26 ++
4 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 40cad9278f..420188a1c6 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -83,6 +83,11 @@ format.outputDirectory::
Set a custom directory to store the resulting files instead of the
current working directory. All directory components will be created.
+format.outputDirectoryCmd::
+ The command which is used to name a custom directory to store the
+ resulting files instead of the current working directory. All directory
+ components will be created.
+
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
format-patch by default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 2035d4d5d5..4936b9f91d 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -67,7 +67,11 @@ can be set with the `format.outputDirectory` configuration
option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
`format.outputDirectory` points elsewhere, use `-o .`. All directory
-components will be created.
+components will be created. The 'format.outputDirectoryCmd' configuration can
+be used to name a command to produce the directory name programmatically. The
+command should produce the name to its standard output. The
+`format.outputDirectory` configuration takes precedence over
+`format.outputDirectoryCmd`.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 8d08632858..3eb507c02f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -774,6 +774,7 @@ static const char *signature = git_version_string;
static const char *signature_file;
static int config_cover_letter;
static const char *config_output_directory;
+static const char *config_output_directory_cmd;
enum {
COVER_UNSET,
@@ -856,6 +857,8 @@ static int git_format_config(const char *var, const char
*value, void *cb)
}
if (!strcmp(var, "format.outputdirectory"))
return git_config_string(&config_output_directory, var, value);
+ if (!strcmp(var, "format.outputdirectorycmd"))
+ return git_config_string(&config_output_directory_cmd, var,
value);
if (!strcmp(var, "format.useautobase")) {
base_auto = git_config_bool(var, value);
return 0;
@@ -1756,8 +1759,27 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
if (rev.show_notes)
init_display_notes(&rev.notes_opt);
- if (!output_directory && !use_stdout)
+ if (!output_directory && !use_stdout) {
+ // outputDirectoryCmd can be preceeded by outputDirectory
+ if (!config_output_directory && config_output_directory_cmd) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ const char *argv[1];
+ struct strbuf buf = STRBUF_INIT;
+ int rc;
+
+ argv[0] = config_output_directory_cmd;
+ cp.argv = argv;
+ cp.use_shell = 1;
+ rc = capture_command(&cp, &buf, PATH_MAX);
+ if (rc)
+ die(_("outputDirectoryCmd command failed: "
+ "'%s'"), config_output_directory_cmd);
+ strbuf_setlen(&buf, strcspn(buf.buf, "\r\n"));
+ config_output_directory = strbuf_detach(&buf, NULL);
+ }
+
output_directory = config_output_directory;
+ }
if (!use_stdout)
output_directory = set_outdir(prefix, output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-fo
[PATCH v5 1/2] format-patch: create leading components of output directory
'git format-patch -o ' did an equivalent of 'mkdir '
not 'mkdir -p ', which is being corrected.
Avoid the usage of 'adjust_shared_perm' on the leading directories which
may have security implications. Achieved by temporarily disabling of
'config.sharedRepository' like 'git init' does.
Signed-off-by: Bert Wesarg
---
Changes in v2:
* squashed and base new tests on 'dl/format-patch-doc-test-cleanup'
Changes in v3:
* avoid applying adjust_shared_perm
Changes in v4:
* based on dl/format-patch-doc-test-cleanup and adopt it
Changes in v5:
* make tests self-contained
Cc: Denton Liu
Cc: Junio C Hamano
Cc: SZEDER Gábor
---
Documentation/config/format.txt| 2 +-
Documentation/git-format-patch.txt | 3 ++-
builtin/log.c | 16
t/t4014-format-patch.sh| 26 ++
4 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index cb629fa769..40cad9278f 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -81,7 +81,7 @@ format.coverLetter::
format.outputDirectory::
Set a custom directory to store the resulting files instead of the
- current working directory.
+ current working directory. All directory components will be created.
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 0ac56f4b70..2035d4d5d5 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -66,7 +66,8 @@ they are created in the current working directory. The
default path
can be set with the `format.outputDirectory` configuration option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
-`format.outputDirectory` points elsewhere, use `-o .`.
+`format.outputDirectory` points elsewhere, use `-o .`. All directory
+components will be created.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 44b10b3415..8d08632858 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1765,10 +1765,26 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
setup_pager();
if (output_directory) {
+ int saved;
if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
rev.diffopt.use_color = GIT_COLOR_NEVER;
if (use_stdout)
die(_("standard output, or directory, which one?"));
+ /*
+* We consider as 'outside of gitdir', therefore avoid
+* applying adjust_shared_perm in s-c-l-d.
+*/
+ saved = get_shared_repository();
+ set_shared_repository(0);
+ switch
(safe_create_leading_directories_const(output_directory)) {
+ case SCLD_OK:
+ case SCLD_EXISTS:
+ break;
+ default:
+ die(_("could not create leading directories "
+ "of '%s'"), output_directory);
+ }
+ set_shared_repository(saved);
if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
die_errno(_("could not create directory '%s'"),
output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 72b09896cf..3aab25da76 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1606,6 +1606,32 @@ test_expect_success 'From line has expected format' '
test_cmp from filtered
'
+test_expect_success 'format-patch -o with no leading directories' '
+ rm -fr patches &&
+ git format-patch -o patches master..side &&
+ count=$(git rev-list --count master..side) &&
+ ls patches >list &&
+ test_line_count = $count list
+'
+
+test_expect_success 'format-patch -o with leading existing directories' '
+ rm -rf existing-dir &&
+ mkdir existing-dir &&
+ git format-patch -o existing-dir/patches master..side &&
+ count=$(git rev-list --count master..side) &&
+ ls existing-dir/patches >list &&
+ test_line_count = $count list
+'
+
+test_expect_success 'format-patch -o with leading non-existing directories' '
+ rm -rf non-existing-dir &&
+ git format-patch -o non-existing-
Re: [PATCH v3 4/4] git-gui: allow undoing last revert
Dear Pratyush,
I just noticed that the 'Revert Last Hunk' menu entry is enabled in
the stage-list. But I think it should be disabled, like the 'Revert
Hunk' and 'Revert Line' menu entry.
Can you confirm this?
Thanks.
Bert
On Wed, Aug 28, 2019 at 11:57 PM Pratyush Yadav wrote:
>
> Accidental clicks on the revert hunk/lines buttons can cause loss of
> work, and can be frustrating. So, allow undoing the last revert.
>
> Right now, a stack or deque are not being used for the sake of
> simplicity, so only one undo is possible. Any reverts before the
> previous one are lost.
>
> Signed-off-by: Pratyush Yadav
> ---
> git-gui.sh | 18 +-
> lib/diff.tcl | 53
> 2 files changed, 66 insertions(+), 5 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 1592544..e03a2d2 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1350,6 +1350,8 @@ set is_submodule_diff 0
> set is_conflict_diff 0
> set selected_commit_type new
> set diff_empty_count 0
> +set last_revert {}
> +set last_revert_enc {}
>
> set nullid ""
> set nullid2 "0001"
> @@ -3601,6 +3603,11 @@ $ctxm add command \
> -command {apply_or_revert_range_or_line $cursorX $cursorY 1;
> do_rescan}
> set ui_diff_revertline [$ctxm index last]
> lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state]
> +$ctxm add command \
> + -label [mc "Undo Last Revert"] \
> + -command {undo_last_revert; do_rescan}
> +set ui_diff_undorevert [$ctxm index last]
> +lappend diff_actions [list $ctxm entryconf $ui_diff_undorevert -state]
> $ctxm add separator
> $ctxm add command \
> -label [mc "Show Less Context"] \
> @@ -3680,7 +3687,7 @@ proc has_textconv {path} {
> }
>
> proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
> - global current_diff_path file_states
> + global current_diff_path file_states last_revert
> set ::cursorX $x
> set ::cursorY $y
> if {[info exists file_states($current_diff_path)]} {
> @@ -3694,6 +3701,7 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
> tk_popup $ctxmsm $X $Y
> } else {
> set has_range [expr {[$::ui_diff tag nextrange sel 0.0] !=
> {}}]
> + set u [mc "Undo Last Revert"]
> if {$::ui_index eq $::current_diff_side} {
> set l [mc "Unstage Hunk From Commit"]
> set h [mc "Revert Hunk"]
> @@ -3739,12 +3747,20 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
> }
> }
>
> + if {$last_revert eq {}} {
> + set undo_state disabled
> + } else {
> + set undo_state normal
> + }
> +
> $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l
> $ctxm entryconf $::ui_diff_applyline -state $s -label $t
> $ctxm entryconf $::ui_diff_revertline -state $revert_state \
> -label $r
> $ctxm entryconf $::ui_diff_reverthunk -state $revert_state \
> -label $h
> + $ctxm entryconf $::ui_diff_undorevert -state $undo_state \
> + -label $u
>
> tk_popup $ctxm $X $Y
> }
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 0659029..96288fc 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -569,7 +569,7 @@ proc read_diff {fd conflict_size cont_info} {
>
> proc apply_or_revert_hunk {x y revert} {
> global current_diff_path current_diff_header current_diff_side
> - global ui_diff ui_index file_states
> + global ui_diff ui_index file_states last_revert last_revert_enc
>
> if {$current_diff_path eq {} || $current_diff_header eq {}} return
> if {![lock_index apply_hunk]} return
> @@ -610,18 +610,25 @@ proc apply_or_revert_hunk {x y revert} {
> set e_lno end
> }
>
> + set wholepatch "$current_diff_header[$ui_diff get $s_lno $e_lno]"
> +
> if {[catch {
> set enc [get_path_encoding $current_diff_path]
> set p [eval git_write $apply_cmd]
> fconfigure $p -translation binary -encoding $enc
> - puts -nonewline $p $current_diff_header
> - puts -nonewline $p [$ui_diff get $s_lno $e_lno]
> + puts -nonewline $p $wholepatch
> close $p} err]} {
> error_popup "$failed_msg\n\n$err"
> unlock_index
> return
> }
>
> + if {$revert} {
> + # Save a copy of this patch for undoing reverts.
> + set last_revert $wholepatch
> + set last_revert_enc $enc
> + }
> +
> $ui_diff conf -state normal
> $ui_diff delete $s_lno $e_lno
> $ui_diff conf
Re: [PATCH v3] git-gui: add a readme
On Fri, Oct 11, 2019 at 11:35 PM Pratyush Yadav wrote: > > I'll take the silence to mean there are no further objections, and will > merge this version in to 'master'. correct. FWIW Acked-by: Bert Wesarg > > On 08/10/19 05:47PM, Pratyush Yadav wrote: > > It is a good idea to have a readme so people finding the project can > > know more about it, and know how they can get involved. > > > > Signed-off-by: Pratyush Yadav > > --- > > Changes in v3: > > - Reword the first paragraph to avoid some repetition. Suggested by > > Junio. > > - Do not mention "written in Tcl" in the intro. Suggested by Junio. > > - Add a list of dependencies. > > -- > Regards, > Pratyush Yadav
Re: [PATCH v4 1/2] format-patch: create leading components of output directory
On Fri, Oct 11, 2019 at 5:45 PM Bert Wesarg wrote: > > On Fri, Oct 11, 2019 at 4:46 PM SZEDER Gábor wrote: > > > > On Fri, Oct 11, 2019 at 10:36:41AM +0200, Bert Wesarg wrote: > > > Changes in v4: > > > * based on dl/format-patch-doc-test-cleanup and adopt it > > > > Thanks... but here I am nitpicking again, sorry :) > > > > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > > index 72b09896cf..9facc3a79e 100755 > > > --- a/t/t4014-format-patch.sh > > > +++ b/t/t4014-format-patch.sh > > > @@ -1606,6 +1606,29 @@ test_expect_success 'From line has expected > > > format' ' > > > test_cmp from filtered > > > ' > > > > > > +test_expect_success 'format-patch -o with no leading directories' ' > > > + rm -fr patches && > > > + git format-patch -o patches master..side && > > > + count=$(git rev-list --count master..side) && > > > + ls patches >list && > > > + test_line_count = $count list > > > +' > > > + > > > +test_expect_success 'format-patch -o with leading existing directories' ' > > > + git format-patch -o patches/side master..side && > > > > The previous test case creates the 'patches' directory and leaves it > > behind, and this test implicitly relies on that directory to check > > that 'format-patch -o' can deal with already existing directories. So > > if the previous test case were to fail early or were not run at all > > (e.g. './t4014-format-patch.sh -r 1,137'), then that directory > > wouldn't exist, and, consequently, this test case would not check what > > it was supposed to. > > > > I think it would be better to be more explicit and self-contained > > about it, and create a leading directory in this test case: > > > >mkdir existing-dir && > >git format-patch -o existing-dir/side master..size && > >ls existing-dir/side >list && one question: How about removing this directory first, just to be sure, that mkdir does create a directory? Bert > > > > thanks. Your nitpicking is always appreciated. > > Bert > > > > + count=$(git rev-list --count master..side) && > > > + ls patches/side >list && > > > + test_line_count = $count list > > > +' > > > + > > > +test_expect_success 'format-patch -o with leading non-existing > > > directories' ' > > > + rm -fr patches && > > > + git format-patch -o patches/side master..side && > > > + count=$(git rev-list --count master..side) && > > > + ls patches/side >list && > > > + test_line_count = $count list > > > +' > > > + > > > test_expect_success 'format-patch format.outputDirectory option' ' > > > test_config format.outputDirectory patches && > > > rm -fr patches && > > > -- > > > 2.23.0.13.g28bc381d7c > > >
Re: [PATCH v4 1/2] format-patch: create leading components of output directory
On Fri, Oct 11, 2019 at 4:46 PM SZEDER Gábor wrote: > > On Fri, Oct 11, 2019 at 10:36:41AM +0200, Bert Wesarg wrote: > > Changes in v4: > > * based on dl/format-patch-doc-test-cleanup and adopt it > > Thanks... but here I am nitpicking again, sorry :) > > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > index 72b09896cf..9facc3a79e 100755 > > --- a/t/t4014-format-patch.sh > > +++ b/t/t4014-format-patch.sh > > @@ -1606,6 +1606,29 @@ test_expect_success 'From line has expected format' ' > > test_cmp from filtered > > ' > > > > +test_expect_success 'format-patch -o with no leading directories' ' > > + rm -fr patches && > > + git format-patch -o patches master..side && > > + count=$(git rev-list --count master..side) && > > + ls patches >list && > > + test_line_count = $count list > > +' > > + > > +test_expect_success 'format-patch -o with leading existing directories' ' > > + git format-patch -o patches/side master..side && > > The previous test case creates the 'patches' directory and leaves it > behind, and this test implicitly relies on that directory to check > that 'format-patch -o' can deal with already existing directories. So > if the previous test case were to fail early or were not run at all > (e.g. './t4014-format-patch.sh -r 1,137'), then that directory > wouldn't exist, and, consequently, this test case would not check what > it was supposed to. > > I think it would be better to be more explicit and self-contained > about it, and create a leading directory in this test case: > >mkdir existing-dir && >git format-patch -o existing-dir/side master..size && >ls existing-dir/side >list && > thanks. Your nitpicking is always appreciated. Bert > > + count=$(git rev-list --count master..side) && > > + ls patches/side >list && > > + test_line_count = $count list > > +' > > + > > +test_expect_success 'format-patch -o with leading non-existing > > directories' ' > > + rm -fr patches && > > + git format-patch -o patches/side master..side && > > + count=$(git rev-list --count master..side) && > > + ls patches/side >list && > > + test_line_count = $count list > > +' > > + > > test_expect_success 'format-patch format.outputDirectory option' ' > > test_config format.outputDirectory patches && > > rm -fr patches && > > -- > > 2.23.0.13.g28bc381d7c > >
[PATCH v4 1/2] format-patch: create leading components of output directory
'git format-patch -o ' did an equivalent of 'mkdir '
not 'mkdir -p ', which is being corrected.
Avoid the usage of 'adjust_shared_perm' on the leading directories which
may have security implications. Achieved by temporarily disabling of
'config.sharedRepository' like 'git init' does.
Signed-off-by: Bert Wesarg
---
Changes in v2:
* squashed and base new tests on 'dl/format-patch-doc-test-cleanup'
Changes in v3:
* avoid applying adjust_shared_perm
Changes in v4:
* based on dl/format-patch-doc-test-cleanup and adopt it
Cc: Denton Liu
Cc: Junio C Hamano
Cc: SZEDER Gábor
---
Documentation/config/format.txt| 2 +-
Documentation/git-format-patch.txt | 3 ++-
builtin/log.c | 16
t/t4014-format-patch.sh| 23 +++
4 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index cb629fa769..40cad9278f 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -81,7 +81,7 @@ format.coverLetter::
format.outputDirectory::
Set a custom directory to store the resulting files instead of the
- current working directory.
+ current working directory. All directory components will be created.
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 0ac56f4b70..2035d4d5d5 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -66,7 +66,8 @@ they are created in the current working directory. The
default path
can be set with the `format.outputDirectory` configuration option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
-`format.outputDirectory` points elsewhere, use `-o .`.
+`format.outputDirectory` points elsewhere, use `-o .`. All directory
+components will be created.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 44b10b3415..8d08632858 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1765,10 +1765,26 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
setup_pager();
if (output_directory) {
+ int saved;
if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
rev.diffopt.use_color = GIT_COLOR_NEVER;
if (use_stdout)
die(_("standard output, or directory, which one?"));
+ /*
+* We consider as 'outside of gitdir', therefore avoid
+* applying adjust_shared_perm in s-c-l-d.
+*/
+ saved = get_shared_repository();
+ set_shared_repository(0);
+ switch
(safe_create_leading_directories_const(output_directory)) {
+ case SCLD_OK:
+ case SCLD_EXISTS:
+ break;
+ default:
+ die(_("could not create leading directories "
+ "of '%s'"), output_directory);
+ }
+ set_shared_repository(saved);
if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
die_errno(_("could not create directory '%s'"),
output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 72b09896cf..9facc3a79e 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1606,6 +1606,29 @@ test_expect_success 'From line has expected format' '
test_cmp from filtered
'
+test_expect_success 'format-patch -o with no leading directories' '
+ rm -fr patches &&
+ git format-patch -o patches master..side &&
+ count=$(git rev-list --count master..side) &&
+ ls patches >list &&
+ test_line_count = $count list
+'
+
+test_expect_success 'format-patch -o with leading existing directories' '
+ git format-patch -o patches/side master..side &&
+ count=$(git rev-list --count master..side) &&
+ ls patches/side >list &&
+ test_line_count = $count list
+'
+
+test_expect_success 'format-patch -o with leading non-existing directories' '
+ rm -fr patches &&
+ git format-patch -o patches/side master..side &&
+ count=$(git rev-list --count master..side) &&
+ ls patches/side >list &&
+ test_line_count = $count list
+'
+
test_expect_success 'format-patch format.outputDirectory option' '
test_config format.outputDirectory patches &&
rm -fr patches &&
--
2.23.0.13.g28bc381d7c
[PATCH v4 2/2] format-patch: configure a command to generate the output directory name
The 'format.outputDirectory' configuration is only able to store constant
directory names. Though some may use
$ git format-patch -o $(createdir) …
to name the directory dynamically. Provide a new configuration to be able
to store such a command too.
Signed-off-by: Bert Wesarg
---
Changes in v2:
* rephrase motivation
Changes in v3:
* remove RFC
Changes in v4:
* based on dl/format-patch-doc-test-cleanup and adopt it
Cc: Alexander Kuleshov
Cc: Eric Sunshine
Cc: Denton Liu
Cc: Junio C Hamano
---
Documentation/config/format.txt| 5 +
Documentation/git-format-patch.txt | 6 +-
builtin/log.c | 24 +++-
t/t4014-format-patch.sh| 26 ++
4 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 40cad9278f..420188a1c6 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -83,6 +83,11 @@ format.outputDirectory::
Set a custom directory to store the resulting files instead of the
current working directory. All directory components will be created.
+format.outputDirectoryCmd::
+ The command which is used to name a custom directory to store the
+ resulting files instead of the current working directory. All directory
+ components will be created.
+
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
format-patch by default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 2035d4d5d5..4936b9f91d 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -67,7 +67,11 @@ can be set with the `format.outputDirectory` configuration
option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
`format.outputDirectory` points elsewhere, use `-o .`. All directory
-components will be created.
+components will be created. The 'format.outputDirectoryCmd' configuration can
+be used to name a command to produce the directory name programmatically. The
+command should produce the name to its standard output. The
+`format.outputDirectory` configuration takes precedence over
+`format.outputDirectoryCmd`.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 8d08632858..3eb507c02f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -774,6 +774,7 @@ static const char *signature = git_version_string;
static const char *signature_file;
static int config_cover_letter;
static const char *config_output_directory;
+static const char *config_output_directory_cmd;
enum {
COVER_UNSET,
@@ -856,6 +857,8 @@ static int git_format_config(const char *var, const char
*value, void *cb)
}
if (!strcmp(var, "format.outputdirectory"))
return git_config_string(&config_output_directory, var, value);
+ if (!strcmp(var, "format.outputdirectorycmd"))
+ return git_config_string(&config_output_directory_cmd, var,
value);
if (!strcmp(var, "format.useautobase")) {
base_auto = git_config_bool(var, value);
return 0;
@@ -1756,8 +1759,27 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
if (rev.show_notes)
init_display_notes(&rev.notes_opt);
- if (!output_directory && !use_stdout)
+ if (!output_directory && !use_stdout) {
+ // outputDirectoryCmd can be preceeded by outputDirectory
+ if (!config_output_directory && config_output_directory_cmd) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ const char *argv[1];
+ struct strbuf buf = STRBUF_INIT;
+ int rc;
+
+ argv[0] = config_output_directory_cmd;
+ cp.argv = argv;
+ cp.use_shell = 1;
+ rc = capture_command(&cp, &buf, PATH_MAX);
+ if (rc)
+ die(_("outputDirectoryCmd command failed: "
+ "'%s'"), config_output_directory_cmd);
+ strbuf_setlen(&buf, strcspn(buf.buf, "\r\n"));
+ config_output_directory = strbuf_detach(&buf, NULL);
+ }
+
output_directory = config_output_directory;
+ }
if (!use_stdout)
output_directory = set_outdir(prefix, output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9facc3a
Re: [PATCH 07/15] t4011: abstract away SHA-1-specific constants
On Tue, Oct 8, 2019 at 2:21 PM Derrick Stolee wrote: > > On 10/5/2019 5:12 PM, brian m. carlson wrote: > > Adjust the test so that it computes variables for object IDs instead of > > using hard-coded hashes. > > [snip] > > > @@ -137,14 +141,17 @@ test_expect_success SYMLINKS 'setup symlinks with > > attributes' ' > > ' > > > > test_expect_success SYMLINKS 'symlinks do not respect userdiff config by > > path' ' > > - cat >expect <<-\EOF && > > + file=$(git rev-parse --short $(git hash-object file.bin)) && > > + link=$(git rev-parse --short \ > > + $(printf file.bin | git hash-object --stdin)) && > > Why this change from $(git hash-object file.bin) to > $(printf file.bin | git hash-object --stdin)? thats two different things. The first hashes the content of file "file.bin". The second hashes the literal string "file.bin". To avoid this confusion, may I suggest to use 'printf "%s" "file.bin"', so that it is clear, that the literal string is meant in this context? Bert > > For that matter, why are you using the "printf|git hash-object" > pattern throughout your change? Seems like an unnecessary hurdle > to me. > > -Stolee
Re: [PATCH v2 1/2] format-patch: create leading components of output directory
On Mon, Oct 7, 2019 at 11:03 PM SZEDER Gábor wrote: > > On Sat, Oct 05, 2019 at 10:43:51AM +0200, Bert Wesarg wrote: > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > index 83f52614d3..2f2cd6fea6 100755 > > --- a/t/t4014-format-patch.sh > > +++ b/t/t4014-format-patch.sh > > @@ -1606,6 +1606,26 @@ test_expect_success 'From line has expected format' ' > > test_cmp from filtered > > ' > > > > +test_expect_success 'format-patch -o with no leading directories' ' > > + rm -fr patches && > > + git format-patch -o patches master..side && > > + git rev-list master..side >list && > > + test_line_count = $(ls patches | wc -l) list > > This is sort of a nit... > > So, these tests check that 'git rev-list ...' lists as many commits as > the number of files created by 'git format-patch'. While it doesn't > affect the tests' correctness, this is subtly different from checking > that 'git format-patch' created as many files as the number of commits > listed by 'git rev-list'. > > Consider how the tests' output would look like on failure: > 'test_line_count' shows an error message that includes the content of > the file to be checked, which in this case would consist of a bunch of > commit object ids: > > test_line_count: line count for list != 3 > f7af51d27933a90554b6e9212a7e5d4ad1c74569 > bd89fce9f5096eb5cad67c342b40818b7e3ce9e4 > > On one hand, these object ids won't mean much to anyone who might have > to debug such a test failure in the future, and on the other these > tests are about 'git format-patch', not about 'git rev-list'. If the > check were written like this: > > count=$(git rev-list --count master..side) && > ls patches >list && > test_line_count = $count list > > then the error message on failure would look something like this: > > test_line_count: line count for list != 3 > 0001-first.patch > 0002-second.patch > > which, I think, would be more useful. thanks for this detail. As I copied an existing test with this pattern, I will add a new pre-patch to this serires which applies your idea to the existing test first, before I add new tests for this patch. Bert > > > > +' > > + > > +test_expect_success 'format-patch -o with leading existing directories' ' > > + git format-patch -o patches/side master..side && > > + git rev-list master..side >list && > > + test_line_count = $(ls patches/side | wc -l) list > > +' > > + > > +test_expect_success 'format-patch -o with leading non-existing > > directories' ' > > + rm -fr patches && > > + git format-patch -o patches/side master..side && > > + git rev-list master..side >list && > > + test_line_count = $(ls patches/side | wc -l) list > > +' > > + > > test_expect_success 'format-patch format.outputDirectory option' ' > > test_config format.outputDirectory patches && > > rm -fr patches && > > -- > > 2.23.0.11.g242cf7f110 > >
Re: [PATCH] git-gui: add a readme
On Sat, Oct 5, 2019 at 12:10 AM Pratyush Yadav wrote: > > It is a good idea to have a readme so people finding the project can > know more about it, and know how they can get involved. > > Signed-off-by: Pratyush Yadav > --- > > I don't have much experience writing this kind of readme or > documentation, so comments are appreciated. Please feel free to chime in > with suggestions and things that can also be added. > > README.md | 128 ++ > 1 file changed, 128 insertions(+) > create mode 100644 README.md > > diff --git a/README.md b/README.md > new file mode 100644 > index 000..d76122d > --- /dev/null > +++ b/README.md > @@ -0,0 +1,128 @@ > +# Git Gui - A graphical user interface for Git > + > +Git Gui is a GUI for [git](https://git-scm.com/) written in Tcl/Tk. It allows > +you to use the git source control management tools via a GUI. This includes > +staging, commiting, adding, pushing, etc. It can also be used as a blame > +viewer, a tree browser, and a citool (make exactly one commit before exiting > +and returning to shell). More details about git-gui can be found in its > manual > +page by either running `man git-gui`, or by visiting the [online manual > +page](https://git-scm.com/docs/git-gui). > + > +Git Gui was initially written by Shawn O. Pearce, and is distributed with the > +standard git installation. > + > +# Building and installing > + > +Most of Git Gui is written in Tcl, so there is not much compilation involved. > +Still, some things do need to be done, so you do need to "build" it. > + > +You can build Git Gui using: > + > +``` > +make > +``` > + > +And then install it using: > + > +``` > +make install > +``` > + > +You probably need to have root/admin permissions to install. > + > +# Contributing > + > +The project is currently maintained by Pratyush Yadav over at > +https://github.com/prati0100/git-gui. Even though the project is hosted at > +GitHub, the development does not happen over GitHub Issues and Pull Requests. > +Instead, an email based workflow is used. The git mailing list > +[git@vger.kernel.org](mailto:git@vger.kernel.org) is where the patches are > +discussed and reviewed. > + > +More information about the git mailing list and instructions to subscribe can > +be found [here](https://git.wiki.kernel.org/index.php/GitCommunity). > + > +## Sending your changes > + > +Since the development happens over email, you need to send in your commits in > +text format. Commits can be converted to emails via the two tools provided by > +git: `git-send-email` and `git-format-patch`. > + > +If you are sending multiple patches, it is recommended to include a cover > +letter. A cover letter is an email explaining in brief what the series is > +supposed to do. A cover letter template can be generated by passing > +`--cover-letter` to `git-format-patch`. > + > +After you send your patches, you might get a review suggesting some changes. > +Make those changes, and re-send your patch(es) in reply to the first patch of > +your initial version. Also please mention the version of the patch. This can > be > +done by passing `-v X` to `git-format-patch`, where 'X' is the version number > +of the patch(es). > + > +### Using git-send-email > + > +You can use `git-send-email` to send patches via email. A pretty good guide > to > +configuring and using `git-send-email` can be found > +[here](https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/) > + > +### Using your email client > + > +If your email client supports sending mbox format emails, you can use > +`git-format-patch` to get an mbox file for each commit, and then send them. > If > +there is more than one patch in the series, then all patches after the first > +patch (or the cover letter) need to be sent as replies to the first. > +`git-send-email` does this by default. > + Junio mentioned (at least?) once [1], that using only git-send-email is not a good workflow. Instead one should use git-format-patch to generate the patches, audit them. and then use git-send-email. I second this. Please switch these two sections to encompass this. Thanks. Bert [1] https://public-inbox.org/git/xmqqh9n241el@gitster.mtv.corp.google.com/ > +### Using GitGitGadget > + > +Since some people prefer a GitHub pull request based workflow, they can use > +[GitGitGadget](https://gitgitgadget.github.io/) to send in patches. The tool > +was originally written for sending patches to the Git project, but it now > also > +supports sending patches for git-gui. > + > +Instructions for using GitGitGadget to send git-gui patches, courtesy of > +Johannes Schindelin: > + > +If you don't already have a fork of the [git/git](https://github.com/git/git) > +repo, you need to make one. Then clone your fork: > + > +``` > +git clone https://github.com//git > +``` > + > +Then add GitGitGadget as a remote: > + > +``` > +git remote add gitgitgadget https://github.com/gitgitgadget/git > +``` > + > +
[PATCH v2 2/2] [RFC] format-patch: configure a command to generate the output directory name
The 'format.outputDirectory' configuration is only able to store constant
directory names. Though some may use
$ git format-patch -o $(createdir) …
to name the directory dynamically. Provide a new configuration to be able
to store such a command too.
Signed-off-by: Bert Wesarg
---
ChangeLog:
v2:
* rephrase motivation
Cc: Alexander Kuleshov
Cc: Eric Sunshine
Cc: Denton Liu
Cc: Junio C Hamano
---
Documentation/config/format.txt| 5 +
Documentation/git-format-patch.txt | 6 +-
builtin/log.c | 24 +++-
t/t4014-format-patch.sh| 25 +
4 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 40cad9278f..420188a1c6 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -83,6 +83,11 @@ format.outputDirectory::
Set a custom directory to store the resulting files instead of the
current working directory. All directory components will be created.
+format.outputDirectoryCmd::
+ The command which is used to name a custom directory to store the
+ resulting files instead of the current working directory. All directory
+ components will be created.
+
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
format-patch by default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 2035d4d5d5..4936b9f91d 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -67,7 +67,11 @@ can be set with the `format.outputDirectory` configuration
option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
`format.outputDirectory` points elsewhere, use `-o .`. All directory
-components will be created.
+components will be created. The 'format.outputDirectoryCmd' configuration can
+be used to name a command to produce the directory name programmatically. The
+command should produce the name to its standard output. The
+`format.outputDirectory` configuration takes precedence over
+`format.outputDirectoryCmd`.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 294534aacb..e841306e61 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -775,6 +775,7 @@ static const char *signature = git_version_string;
static const char *signature_file;
static int config_cover_letter;
static const char *config_output_directory;
+static const char *config_output_directory_cmd;
enum {
COVER_UNSET,
@@ -857,6 +858,8 @@ static int git_format_config(const char *var, const char
*value, void *cb)
}
if (!strcmp(var, "format.outputdirectory"))
return git_config_string(&config_output_directory, var, value);
+ if (!strcmp(var, "format.outputdirectorycmd"))
+ return git_config_string(&config_output_directory_cmd, var,
value);
if (!strcmp(var, "format.useautobase")) {
base_auto = git_config_bool(var, value);
return 0;
@@ -1757,8 +1760,27 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
if (rev.show_notes)
init_display_notes(&rev.notes_opt);
- if (!output_directory && !use_stdout)
+ if (!output_directory && !use_stdout) {
+ // outputDirectoryCmd can be preceeded by outputDirectory
+ if (!config_output_directory && config_output_directory_cmd) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ const char *argv[1];
+ struct strbuf buf = STRBUF_INIT;
+ int rc;
+
+ argv[0] = config_output_directory_cmd;
+ cp.argv = argv;
+ cp.use_shell = 1;
+ rc = capture_command(&cp, &buf, PATH_MAX);
+ if (rc)
+ die(_("outputDirectoryCmd command failed: "
+ "'%s'"), config_output_directory_cmd);
+ strbuf_setlen(&buf, strcspn(buf.buf, "\r\n"));
+ config_output_directory = strbuf_detach(&buf, NULL);
+ }
+
output_directory = config_output_directory;
+ }
if (!use_stdout)
output_directory = set_outdir(prefix, output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 2f2cd6fea6..f351d26543 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1642,6 +1642,3
[PATCH v2 1/2] format-patch: create leading components of output directory
Signed-off-by: Bert Wesarg
---
ChangeLog:
v2:
* squashed and base new tests on 'dl/format-patch-doc-test-cleanup'
Cc: Denton Liu
---
Documentation/config/format.txt| 2 +-
Documentation/git-format-patch.txt | 3 ++-
builtin/log.c | 8
t/t4014-format-patch.sh| 20
4 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index cb629fa769..40cad9278f 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -81,7 +81,7 @@ format.coverLetter::
format.outputDirectory::
Set a custom directory to store the resulting files instead of the
- current working directory.
+ current working directory. All directory components will be created.
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 0ac56f4b70..2035d4d5d5 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -66,7 +66,8 @@ they are created in the current working directory. The
default path
can be set with the `format.outputDirectory` configuration option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
-`format.outputDirectory` points elsewhere, use `-o .`.
+`format.outputDirectory` points elsewhere, use `-o .`. All directory
+components will be created.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index c4b35fdaf9..294534aacb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1770,6 +1770,14 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
rev.diffopt.use_color = GIT_COLOR_NEVER;
if (use_stdout)
die(_("standard output, or directory, which one?"));
+ switch
(safe_create_leading_directories_const(output_directory)) {
+ case SCLD_OK:
+ case SCLD_EXISTS:
+ break;
+ default:
+ die(_("could not create leading directories "
+ "of '%s'"), output_directory);
+ }
if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
die_errno(_("could not create directory '%s'"),
output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 83f52614d3..2f2cd6fea6 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1606,6 +1606,26 @@ test_expect_success 'From line has expected format' '
test_cmp from filtered
'
+test_expect_success 'format-patch -o with no leading directories' '
+ rm -fr patches &&
+ git format-patch -o patches master..side &&
+ git rev-list master..side >list &&
+ test_line_count = $(ls patches | wc -l) list
+'
+
+test_expect_success 'format-patch -o with leading existing directories' '
+ git format-patch -o patches/side master..side &&
+ git rev-list master..side >list &&
+ test_line_count = $(ls patches/side | wc -l) list
+'
+
+test_expect_success 'format-patch -o with leading non-existing directories' '
+ rm -fr patches &&
+ git format-patch -o patches/side master..side &&
+ git rev-list master..side >list &&
+ test_line_count = $(ls patches/side | wc -l) list
+'
+
test_expect_success 'format-patch format.outputDirectory option' '
test_config format.outputDirectory patches &&
rm -fr patches &&
--
2.23.0.11.g242cf7f110
Re: [PATCH 1/3] format-patch: document and exercise that -o does only create the trailing directory
Denton, On Wed, Oct 2, 2019 at 11:47 PM Denton Liu wrote: > > Hi Bert, > > > Subject: format-patch: document and exercise that -o does only create the > > trailing directory > > s/does only create/only creates/ ? > > Anyway, as a prepatory patch, I don't think that it's necessary. Maybe > it's just me but I assume that most tools create at most one directory > deep. Even mkdir won't created nested dirs unless you pass `-p`. I > dunno. > > On Wed, Oct 02, 2019 at 11:26:11PM +0200, Bert Wesarg wrote: > > Signed-off-by: Bert Wesarg > > --- > > Documentation/config/format.txt| 3 ++- > > Documentation/git-format-patch.txt | 4 +++- > > t/t4014-format-patch.sh| 16 > > 3 files changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/config/format.txt > > b/Documentation/config/format.txt > > index 414a5a8a9d..e17c5d6b0f 100644 > > --- a/Documentation/config/format.txt > > +++ b/Documentation/config/format.txt > > @@ -80,7 +80,8 @@ format.coverLetter:: > > > > format.outputDirectory:: > > Set a custom directory to store the resulting files instead of the > > - current working directory. > > + current working directory. Only the trailing directory will be created > > + though. > > > > format.useAutoBase:: > > A boolean value which lets you enable the `--base=auto` option of > > diff --git a/Documentation/git-format-patch.txt > > b/Documentation/git-format-patch.txt > > index b9b97e63ae..fe7492353e 100644 > > --- a/Documentation/git-format-patch.txt > > +++ b/Documentation/git-format-patch.txt > > @@ -66,7 +66,9 @@ they are created in the current working directory. The > > default path > > can be set with the `format.outputDirectory` configuration option. > > The `-o` option takes precedence over `format.outputDirectory`. > > To store patches in the current working directory even when > > -`format.outputDirectory` points elsewhere, use `-o .`. > > +`format.outputDirectory` points elsewhere, use `-o .`. Note that only > > +the trailing directory will be created by Git, leading directories must > > +already exists. > > > > By default, the subject of a single patch is "[PATCH] " followed by > > the concatenation of lines from the commit message up to the first blank > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > index ca7debf1d4..bf2715a503 100755 > > --- a/t/t4014-format-patch.sh > > +++ b/t/t4014-format-patch.sh > > @@ -1632,6 +1632,22 @@ test_expect_success 'From line has expected format' ' > > test_cmp from filtered > > ' > > > > +test_expect_success 'format-patch -o with no leading directories' ' > > + rm -fr patches && > > + git format-patch -o patches master..side && > > + test $(git rev-list master..side | wc -l) -eq $(ls patches | wc -l) > > For test case you write, please use the following pattern: > > git rev-list master..side >list && > test_line_count = $(ls patches | wc -l) list > > The first benefit is that we get to take advantage of the > test_line_count function that's already written for us. The second is > that when we write tests, we shouldn't put Git commands in the upstream > of a pipe because if they fail, their return codes will be lost and we > won't be able to fail the test properly. thanks. I will ensure, that this follows your dl/format-patch-doc-test-cleanup topic. Bert > > > +' > > + > > +test_expect_success 'format-patch -o with leading existing directories' ' > > + git format-patch -o patches/side master..side && > > + test $(git rev-list master..side | wc -l) -eq $(ls patches/side | wc > > -l) > > +' > > + > > +test_expect_failure 'format-patch -o with leading non-existing > > directories' ' > > + rm -fr patches && > > + git format-patch -o patches/side master..side > > +' > > As above, I wouldn't really call this a bug in Git. I think we should > leave this test case off until the next patch. > > > + > > test_expect_success 'format-patch format.outputDirectory option' ' > > test_config format.outputDirectory patches && > > rm -fr patches && > > -- > > 2.23.0.11.g242cf7f110 > >
[PATCH 2/3] format-patch: create output directory including leading components
Signed-off-by: Bert Wesarg
---
Documentation/config/format.txt| 3 +--
Documentation/git-format-patch.txt | 5 ++---
builtin/log.c | 8
t/t4014-format-patch.sh| 5 +++--
4 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index e17c5d6b0f..b6c96ece04 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -80,8 +80,7 @@ format.coverLetter::
format.outputDirectory::
Set a custom directory to store the resulting files instead of the
- current working directory. Only the trailing directory will be created
- though.
+ current working directory. All directory components will be created.
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index fe7492353e..f418f490aa 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -66,9 +66,8 @@ they are created in the current working directory. The
default path
can be set with the `format.outputDirectory` configuration option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
-`format.outputDirectory` points elsewhere, use `-o .`. Note that only
-the trailing directory will be created by Git, leading directories must
-already exists.
+`format.outputDirectory` points elsewhere, use `-o .`. All directory
+components will be created.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 44b10b3415..1ab9eb6b78 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1769,6 +1769,14 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
rev.diffopt.use_color = GIT_COLOR_NEVER;
if (use_stdout)
die(_("standard output, or directory, which one?"));
+ switch
(safe_create_leading_directories_const(output_directory)) {
+ case SCLD_OK:
+ case SCLD_EXISTS:
+ break;
+ default:
+ die(_("could not create leading directories "
+ "of '%s'"), output_directory);
+ }
if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
die_errno(_("could not create directory '%s'"),
output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index bf2715a503..43d608aa94 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1643,9 +1643,10 @@ test_expect_success 'format-patch -o with leading
existing directories' '
test $(git rev-list master..side | wc -l) -eq $(ls patches/side | wc -l)
'
-test_expect_failure 'format-patch -o with leading non-existing directories' '
+test_expect_success 'format-patch -o with leading non-existing directories' '
rm -fr patches &&
- git format-patch -o patches/side master..side
+ git format-patch -o patches/side master..side &&
+ test $(git rev-list master..side | wc -l) -eq $(ls patches/side | wc -l)
'
test_expect_success 'format-patch format.outputDirectory option' '
--
2.23.0.11.g242cf7f110
[RFC PATCH 3/3] format-patch: use a command to generate the output directory name
Having 'format.outputDirectory' is convenient, but being able to process
all produced patches via a wildcard command is even more so. I.e.,
using an argument like '/*'. Neither '-o' nor
'format.outputDirectory' can be parameterized to produce a new unique
directory. Thus provide the new 'format.outputDirectoryCmd' configuration
to specify a command which does the job and puts the name to standard
output.
Signed-off-by: Bert Wesarg
---
Cc: Alexander Kuleshov
Cc: Eric Sunshine
---
Documentation/config/format.txt| 5 +
Documentation/git-format-patch.txt | 6 +-
builtin/log.c | 24 +++-
t/t4014-format-patch.sh| 24
4 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index b6c96ece04..dcce2c67ef 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -82,6 +82,11 @@ format.outputDirectory::
Set a custom directory to store the resulting files instead of the
current working directory. All directory components will be created.
+format.outputDirectoryCmd::
+ The command which is used to name a custom directory to store the
+ resulting files instead of the current working directory. All directory
+ components will be created.
+
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
format-patch by default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index f418f490aa..0da904255b 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -67,7 +67,11 @@ can be set with the `format.outputDirectory` configuration
option.
The `-o` option takes precedence over `format.outputDirectory`.
To store patches in the current working directory even when
`format.outputDirectory` points elsewhere, use `-o .`. All directory
-components will be created.
+components will be created. The 'format.outputDirectoryCmd' configuration can
+be used to name a command to produce the directory name programmatically. The
+command should produce the name to its standard output. The
+`format.outputDirectory` configuration takes precedence over
+`format.outputDirectoryCmd`.
By default, the subject of a single patch is "[PATCH] " followed by
the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 1ab9eb6b78..b102e86bea 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -774,6 +774,7 @@ static const char *signature = git_version_string;
static const char *signature_file;
static int config_cover_letter;
static const char *config_output_directory;
+static const char *config_output_directory_cmd;
enum {
COVER_UNSET,
@@ -856,6 +857,8 @@ static int git_format_config(const char *var, const char
*value, void *cb)
}
if (!strcmp(var, "format.outputdirectory"))
return git_config_string(&config_output_directory, var, value);
+ if (!strcmp(var, "format.outputdirectorycmd"))
+ return git_config_string(&config_output_directory_cmd, var,
value);
if (!strcmp(var, "format.useautobase")) {
base_auto = git_config_bool(var, value);
return 0;
@@ -1756,8 +1759,27 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
if (rev.show_notes)
init_display_notes(&rev.notes_opt);
- if (!output_directory && !use_stdout)
+ if (!output_directory && !use_stdout) {
+ // outputDirectoryCmd can be preceeded by outputDirectory
+ if (!config_output_directory && config_output_directory_cmd) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ const char *argv[1];
+ struct strbuf buf = STRBUF_INIT;
+ int rc;
+
+ argv[0] = config_output_directory_cmd;
+ cp.argv = argv;
+ cp.use_shell = 1;
+ rc = capture_command(&cp, &buf, PATH_MAX);
+ if (rc)
+ die(_("outputDirectoryCmd command failed: "
+ "'%s'"), config_output_directory_cmd);
+ strbuf_setlen(&buf, strcspn(buf.buf, "\r\n"));
+ config_output_directory = strbuf_detach(&buf, NULL);
+ }
+
output_directory = config_output_directory;
+ }
if (!use_stdout)
output_directory = set_outdir(prefix, output_directory);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.
[PATCH 1/3] format-patch: document and exercise that -o does only create the trailing directory
Signed-off-by: Bert Wesarg --- Documentation/config/format.txt| 3 ++- Documentation/git-format-patch.txt | 4 +++- t/t4014-format-patch.sh| 16 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt index 414a5a8a9d..e17c5d6b0f 100644 --- a/Documentation/config/format.txt +++ b/Documentation/config/format.txt @@ -80,7 +80,8 @@ format.coverLetter:: format.outputDirectory:: Set a custom directory to store the resulting files instead of the - current working directory. + current working directory. Only the trailing directory will be created + though. format.useAutoBase:: A boolean value which lets you enable the `--base=auto` option of diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index b9b97e63ae..fe7492353e 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -66,7 +66,9 @@ they are created in the current working directory. The default path can be set with the `format.outputDirectory` configuration option. The `-o` option takes precedence over `format.outputDirectory`. To store patches in the current working directory even when -`format.outputDirectory` points elsewhere, use `-o .`. +`format.outputDirectory` points elsewhere, use `-o .`. Note that only +the trailing directory will be created by Git, leading directories must +already exists. By default, the subject of a single patch is "[PATCH] " followed by the concatenation of lines from the commit message up to the first blank diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index ca7debf1d4..bf2715a503 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1632,6 +1632,22 @@ test_expect_success 'From line has expected format' ' test_cmp from filtered ' +test_expect_success 'format-patch -o with no leading directories' ' + rm -fr patches && + git format-patch -o patches master..side && + test $(git rev-list master..side | wc -l) -eq $(ls patches | wc -l) +' + +test_expect_success 'format-patch -o with leading existing directories' ' + git format-patch -o patches/side master..side && + test $(git rev-list master..side | wc -l) -eq $(ls patches/side | wc -l) +' + +test_expect_failure 'format-patch -o with leading non-existing directories' ' + rm -fr patches && + git format-patch -o patches/side master..side +' + test_expect_success 'format-patch format.outputDirectory option' ' test_config format.outputDirectory patches && rm -fr patches && -- 2.23.0.11.g242cf7f110
[PATCH v3 2/2] git-gui: support for diff3 conflict style
This adds highlight support for the diff3 conflict style.
The common pre-image will be reversed to --, because it has been removed
and replaced with ours or theirs side respectively.
Signed-off-by: Bert Wesarg
---
git-gui.sh | 3 +++
lib/diff.tcl | 17 -
2 files changed, 19 insertions(+), 1 deletion(-)
---
v3: Fixed a syntax error
diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..6d80f82 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
$ui_diff tag conf d< \
-foreground orange \
-font font_diffbold
+$ui_diff tag conf d| \
+ -foreground orange \
+ -font font_diffbold
$ui_diff tag conf d= \
-foreground orange \
-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0fd4600..dacdda2 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -347,6 +347,10 @@ proc start_show_diff {cont_info {add_opts {}}} {
}
set ::current_diff_inheader 1
+ # detect pre-image lines of the diff3 conflict-style, they are just '++'
+ # lines which is not bijective, thus we need to maintain a state across
+ # lines
+ set ::conflict_in_pre_image 0
fconfigure $fd \
-blocking 0 \
-encoding [get_path_encoding $path] \
@@ -449,11 +453,22 @@ proc read_diff {fd conflict_size cont_info} {
{--} {set tags d_--}
{++} {
set regexp [string map [list %conflict_size
$conflict_size]\
-
{^\+\+([<>=]){%conflict_size}(?: |$)}]
+
{^\+\+([<>=|]){%conflict_size}(?: |$)}]
if {[regexp $regexp $line _g op]} {
set is_conflict_diff 1
set line [string replace $line 0 1 { }]
set tags d$op
+ # the ||| conflict-marker marks the
start of the pre-image,
+ # all those lines are also prefixed
with '++', thus we need
+ # to maintain this state
+ set ::conflict_in_pre_image [expr {$op
eq {|}}]
+ } elseif {$::conflict_in_pre_image} {
+ # this is a pre-image line, it is the
one which both sides
+ # are based on. As it has also the '++'
line start, it is
+ # normally shown as 'added', invert
this to '--' to make
+ # it a 'removed' line
+ set line [string replace $line 0 1 {--}]
+ set tags d_--
} else {
set tags d_++
}
--
2.23.0.11.g242cf7f110
Re: [PATCH v3 1/2] git-gui: use existing interface to query a path's attribute
Pratyush, On Tue, Oct 1, 2019 at 7:31 PM Pratyush Yadav wrote: > > On 01/10/19 05:22PM, Bert Wesarg wrote: > > On Tue, Oct 1, 2019 at 4:24 PM Pratyush Yadav > > wrote: > > > > > > Hi, > > > > > > I don't see any difference between v3 and v2 of this patch. What changed > > > in this version? > > > > nothing, but 2/2 changed. > > I don't see a v3 of 2/2 in my inbox. A search on public-inbox yields > nothing either. Can you please check if the patch was sent properly? Or > if you _can_ find it on public-inbox.org, a link to that would do just > fine. > > I _do_ have v2 of both patches, but the v3 of the second patch is the > one missing. I noticed this already, while in contact with Johannes on GitHub and I found my error. While pasting messages ids for References into the v3 patch, I missed to renamed Message-Id to References for the v2 patch, thus the v3 has the same Message-Id than the v2 patch. Will resend now. Bert > > -- > Regards, > Pratyush Yadav
Re: [PATCH v3 1/2] git-gui: use existing interface to query a path's attribute
On Tue, Oct 1, 2019 at 4:24 PM Pratyush Yadav wrote:
>
> Hi,
>
> I don't see any difference between v3 and v2 of this patch. What changed
> in this version?
nothing, but 2/2 changed.
Bert
>
> On 30/09/19 09:54PM, Bert Wesarg wrote:
> > Replace the hand-coded call to git check-attr with the already provided one.
> >
> > Signed-off-by: Bert Wesarg
> > ---
> > lib/diff.tcl | 15 +--
> > 1 file changed, 1 insertion(+), 14 deletions(-)
> >
> > diff --git a/lib/diff.tcl b/lib/diff.tcl
> > index 958a0fa..0fd4600 100644
> > --- a/lib/diff.tcl
> > +++ b/lib/diff.tcl
> > @@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
> > }
> > }
> >
> > -proc get_conflict_marker_size {path} {
> > - set size 7
> > - catch {
> > - set fd_rc [eval [list git_read check-attr
> > "conflict-marker-size" -- $path]]
> > - set ret [gets $fd_rc line]
> > - close $fd_rc
> > - if {$ret > 0} {
> > - regexp {.*: conflict-marker-size: (\d+)$} $line line
> > size
> > - }
> > - }
> > - return $size
> > -}
> > -
> > proc start_show_diff {cont_info {add_opts {}}} {
> > global file_states file_lists
> > global is_3way_diff is_submodule_diff diff_active repo_config
> > @@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
> > set is_submodule_diff 0
> > set diff_active 1
> > set current_diff_header {}
> > - set conflict_size [get_conflict_marker_size $path]
> > + set conflict_size [gitattr $path conflict-marker-size 7]
> >
> > set cmd [list]
> > if {$w eq $ui_index} {
> > --
> > 2.23.0.11.g242cf7f110
> >
>
> --
> Regards,
> Pratyush Yadav
Re: git-grep in sparse checkout
Hi, On Tue, Oct 1, 2019 at 3:06 PM Matheus Tavares Bernardino wrote: > > Hi, > > During Git Summit it was mentioned that git-grep searches outside > sparsity pattern which is not aligned with user expectation. I took a > quick look at it and it seems the reason is > builtin/grep.c:grep_cache() (which also greps worktree) will grep the > object store when a given index entry has the CE_SKIP_WORKTREE bit > turned on. > I also had once this problem and found that out and wrote a patch. I was just about to send this patch out. Btw, ls-files should also learn to skip worktree files. Stay tuned. Bert > From what I understand, this bit is used exactly for sparse checkouts > (as described in Documentation/technical/index-format.txt[1]). But > should we perhaps ignore it in git-grep to have the expected behavior? > I'll be happy to send the patch if so, but I wanted to check with you > first. > > Grepping with --cached or in given trees objects would still grep > outside the cone, but that is what one would expect, right? Or should > we filter out what is outside of the cone for cached grep as well? > > Thanks, > Matheus > > [1]: > https://github.com/git/git/blob/master/Documentation/technical/index-format.txt#L101
[PATCH] gitk: Add horizontal scrollbar to the files list
Wrapping filenames is an unexpected experience in UX design. Disable
wrapping and add a horizontal scrollbar to the files list to remove this.
Signed-off-by: Bert Wesarg
---
gitk | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/gitk b/gitk
index abe4805..bf2a061 100755
--- a/gitk
+++ b/gitk
@@ -2477,13 +2477,16 @@ proc makewindow {} {
-background $bgcolor -foreground $fgcolor \
-font mainfont \
-tabs [list $indent [expr {2 * $indent}]] \
- -yscrollcommand ".bright.sb set" \
+ -xscrollcommand ".bright.sbx set" \
+ -yscrollcommand ".bright.sby set" \
-cursor [. cget -cursor] \
- -spacing1 1 -spacing3 1
+ -spacing1 1 -spacing3 1 -wrap none
lappend bglist $cflist
lappend fglist $cflist
-${NS}::scrollbar .bright.sb -command "$cflist yview"
-pack .bright.sb -side right -fill y
+${NS}::scrollbar .bright.sbx -orient horizontal -command "$cflist xview"
+${NS}::scrollbar .bright.sby -orient vertical -command "$cflist yview"
+pack .bright.sbx -side bottom -fill x
+pack .bright.sby -side right -fill y
pack $cflist -side left -fill both -expand 1
$cflist tag configure highlight \
-background [$cflist cget -selectbackground]
--
2.23.0.11.g242cf7f110
[PATCH v3 1/2] git-gui: use existing interface to query a path's attribute
Replace the hand-coded call to git check-attr with the already provided one.
Signed-off-by: Bert Wesarg
---
lib/diff.tcl | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 958a0fa..0fd4600 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
}
}
-proc get_conflict_marker_size {path} {
- set size 7
- catch {
- set fd_rc [eval [list git_read check-attr
"conflict-marker-size" -- $path]]
- set ret [gets $fd_rc line]
- close $fd_rc
- if {$ret > 0} {
- regexp {.*: conflict-marker-size: (\d+)$} $line line
size
- }
- }
- return $size
-}
-
proc start_show_diff {cont_info {add_opts {}}} {
global file_states file_lists
global is_3way_diff is_submodule_diff diff_active repo_config
@@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
set is_submodule_diff 0
set diff_active 1
set current_diff_header {}
- set conflict_size [get_conflict_marker_size $path]
+ set conflict_size [gitattr $path conflict-marker-size 7]
set cmd [list]
if {$w eq $ui_index} {
--
2.23.0.11.g242cf7f110
[PATCH v2 2/2] git-gui: support for diff3 conflict style
This adds highlight support for the diff3 conflict style.
The common pre-image will be reversed to --, because it has been removed
and replaced with ours or theirs side respectively.
Signed-off-by: Bert Wesarg
---
git-gui.sh | 3 +++
lib/diff.tcl | 17 -
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..6d80f82 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
$ui_diff tag conf d< \
-foreground orange \
-font font_diffbold
+$ui_diff tag conf d| \
+ -foreground orange \
+ -font font_diffbold
$ui_diff tag conf d= \
-foreground orange \
-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0fd4600..6459cfb 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -347,6 +347,10 @@ proc start_show_diff {cont_info {add_opts {}}} {
}
set ::current_diff_inheader 1
+ # detect pre-image lines of the diff3 conflict-style, they are just '++'
+ # lines which is not bijective, thus we need to maintain a state across
+ # lines
+ set ::conflict_in_pre_image 0
fconfigure $fd \
-blocking 0 \
-encoding [get_path_encoding $path] \
@@ -449,11 +453,22 @@ proc read_diff {fd conflict_size cont_info} {
{--} {set tags d_--}
{++} {
set regexp [string map [list %conflict_size
$conflict_size]\
-
{^\+\+([<>=]){%conflict_size}(?: |$)}]
+
{^\+\+([<>=|]){%conflict_size}(?: |$)}]
if {[regexp $regexp $line _g op]} {
set is_conflict_diff 1
set line [string replace $line 0 1 { }]
set tags d$op
+ # the ||| conflict-marker marks the
start of the pre-image,
+ # all those lines are also prefixed
with '++', thus we need
+ # to maintain this state
+ set ::conflict_in_pre_image [expr {$op
eq {|}}
+ } elseif {$::conflict_in_pre_image} {
+ # this is a pre-image line, it is the
one which both sides
+ # are based on. As it has also the '++'
line start, it is
+ # normally shown as 'added', invert
this to '--' to make
+ # it a 'removed' line
+ set line [string replace $line 0 1 {--}]
+ set tags d_--
} else {
set tags d_++
}
--
2.23.0.11.g242cf7f110
[PATCH v2 1/2] git-gui: use existing interface to query a path's attribute
Replace the hand-coded call to git check-attr with the already provided one.
Signed-off-by: Bert Wesarg
---
lib/diff.tcl | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 958a0fa..0fd4600 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
}
}
-proc get_conflict_marker_size {path} {
- set size 7
- catch {
- set fd_rc [eval [list git_read check-attr
"conflict-marker-size" -- $path]]
- set ret [gets $fd_rc line]
- close $fd_rc
- if {$ret > 0} {
- regexp {.*: conflict-marker-size: (\d+)$} $line line
size
- }
- }
- return $size
-}
-
proc start_show_diff {cont_info {add_opts {}}} {
global file_states file_lists
global is_3way_diff is_submodule_diff diff_active repo_config
@@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
set is_submodule_diff 0
set diff_active 1
set current_diff_header {}
- set conflict_size [get_conflict_marker_size $path]
+ set conflict_size [gitattr $path conflict-marker-size 7]
set cmd [list]
if {$w eq $ui_index} {
--
2.23.0.11.g242cf7f110
Re: [PATCH 2/2] git-gui: support for diff3 conflict style
Pratyush,
On Sun, Sep 29, 2019 at 5:04 PM Pratyush Yadav wrote:
>
> Hi Philip, Bert,
>
> Is there any way I can test this change? Philip, I ran the rebase you
> mention in the GitHub issue [0], and I get that '9c8cba6862abe5ac821' is
> an unknown revision.
>
> Is there any quick way I can reproduce this (maybe on a sample repo)?
The easiest way to produce a merge conflict, is to change the same
line differently in two branches and try to merge them. I added a
fast-import file to demonstrate this for you.
$ git init foo
$ cd foo
$ git fast-import <../conflict-merge.fi
$ git reset --hard master
$ git merge branch
this gets you into the conflict, probably the usual style. Which looks
in liek this on the terminal:
@@@ -2,7 -2,7 +2,11 @@@ Lorem ipsum dolor sit amet, consectetu
Sed feugiat nisl eget efficitur ultrices.
Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
++<<<<<<< HEAD
+Proin bibendum purus ut est tristique, non pharetra dui consectetur.
++===
+ Proin placerat leo malesuada lacinia lobortis.
++>>>>>>> branch
Pellentesque aliquam libero et nisi scelerisque commodo.
Quisque id velit sed magna molestie porttitor.
Vivamus sed urna in risus molestie ultricies.
and this in git gui: https://kgab.selfhost.eu/s/gHHaQqowGp7mXEb
Git gui removes the '++' in front of the marker lines. It therefor
already 'changes' the 'diff'. Though git-apply cannot handle such
'diffs' anyway.
To get the diff3 style do:
$ git merge --abort
$ git -c merge.conflictStyle=diff3 merge branch
This is how it looks in the terminal now:
@@@ -2,7 -2,7 +2,13 @@@ Lorem ipsum dolor sit amet, consectetu
Sed feugiat nisl eget efficitur ultrices.
Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
++<<<<<<< HEAD
+Proin bibendum purus ut est tristique, non pharetra dui consectetur.
++||| merged common ancestors
++Proin in felis eu elit suscipit rhoncus vel ut metus.
++===
+ Proin placerat leo malesuada lacinia lobortis.
++>>>>>>> branch
Pellentesque aliquam libero et nisi scelerisque commodo.
Quisque id velit sed magna molestie porttitor.
Vivamus sed urna in risus molestie ultricies.
As you can see, there is not the usual 'I removed this, and added
that' experience, everything is 'added'. Thus I inverted the pre-image
to '--'. Here is how it looks in the gui:
https://kgab.selfhost.eu/s/5c8Tosra7WRfjwJ
> [0] https://github.com/git-for-windows/git/issues/2340
>
> On 25/09/19 10:38PM, Bert Wesarg wrote:
> > This adds highlight support for the diff3 conflict style.
> >
> > The common pre-image will be reversed to --, because it has been removed
> > and either replaced with ours or theirs side.
> >
> > Signed-off-by: Bert Wesarg
> > ---
> > git-gui.sh | 3 +++
> > lib/diff.tcl | 22 ++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index fd476b6..6d80f82 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
> > $ui_diff tag conf d< \
> > -foreground orange \
> > -font font_diffbold
> > +$ui_diff tag conf d| \
> > + -foreground orange \
> > + -font font_diffbold
> > $ui_diff tag conf d= \
> > -foreground orange \
> > -font font_diffbold
> > diff --git a/lib/diff.tcl b/lib/diff.tcl
> > index 0fd4600..6caf4e7 100644
> > --- a/lib/diff.tcl
> > +++ b/lib/diff.tcl
> > @@ -347,6 +347,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
> > }
> >
> > set ::current_diff_inheader 1
> > + set ::conflict_state {CONTEXT}
> > fconfigure $fd \
> > -blocking 0 \
> > -encoding [get_path_encoding $path] \
> > @@ -450,10 +451,28 @@ proc read_diff {fd conflict_size cont_info} {
> > {++} {
> > set regexp [string map [list %conflict_size
> > $conflict_size]\
> >
> > {^\+\+([<>=]){%conflict_size}(?: |$)}]
> > + set regexp_pre_image [string map [list
> > %conflict_size $conflict_size]\
> > +
> > {^\+\+\|{%conflict_size}(?: |$)}]
> > if {[regexp $regexp $line _g op]} {
> > set is_conflic
[PATCH] builtin/submodule--helper: fix usage string for 'update-clone'
Signed-off-by: Bert Wesarg
---
builtin/submodule--helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 909e77e802..89d6f02969 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1874,7 +1874,7 @@ static int update_clone(int argc, const char **argv,
const char *prefix)
};
const char *const git_submodule_helper_usage[] = {
- N_("git submodule--helper update_clone [--prefix=]
[...]"),
+ N_("git submodule--helper update-clone [--prefix=]
[...]"),
NULL
};
suc.prefix = prefix;
--
2.23.0.14.g3fe90158fc
Re: [PATCH 1/1] respect core.hooksPath, falling back to .git/hooks
On Fri, Sep 27, 2019 at 12:40 AM Pratyush Yadav wrote:
>
> Hi,
>
> On 26/09/19 02:17PM, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin
> >
> > Since v2.9.0, Git knows about the config variable core.hookspath
> > that allows overriding the path to the directory containing the
> > Git hooks.
> >
> > Since v2.10.0, the `--git-path` option respects that config
> > variable, too, so we may just as well use that command.
> >
> > For Git versions older than v2.5.0 (which was the first version to
> > support the `--git-path` option for the `rev-parse` command), we
> > simply fall back to the previous code.
> >
> > This fixes https://github.com/git-for-windows/git/issues/1755
> >
> > Initial-patch-by: Philipp Gortan
> > Signed-off-by: Johannes Schindelin
> > ---
> > git-gui.sh | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index fd476b6999..b2c6e7a1db 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -623,7 +623,11 @@ proc git_write {args} {
> > }
> >
> > proc githook_read {hook_name args} {
> > - set pchook [gitdir hooks $hook_name]
> > + if {[package vcompare $::_git_version 2.5.0] >= 0} {
> > + set pchook [git rev-parse --git-path "hooks/$hook_name"]
> > + } else {
> > + set pchook [gitdir hooks $hook_name]
> > + }
>
> gitdir is used in a lot of places, and I think all those would also
> benefit from using --git-path. So I think it is a better idea to move
> this to the procedure gitdir. It would have to be refactored to take any
> number of arguments, instead of the two it takes here.
gitdir already takes an arbitrary number of arguments and joins them
to a path. The more imminent challenge is, that gitdir caches the
GIT_DIR, thus it tries to avoid calling "git rev-parse". Which works
for most, but not for hooks.
We could either maintain a blacklist, for what we cache the result
too, or always call "git rev-parse --git-dir".
This blacklist would need to be in sync with the one in Git's
path.c::adjust_git_path() than.
Bert
>
> Other than that, looks good. Thanks.
>
> --
> Regards,
> Pratyush Yadav
Re: [PATCH 1/1] git gui: fix staging a second line to a 1-line file
On Thu, Sep 26, 2019 at 7:43 PM Johannes Schindelin via GitGitGadget
wrote:
>
> From: Johannes Schindelin
>
> When a 1-line file is augmented by a second line, and the user tries to
> stage that single line via the "Stage Line" context menu item, we do not
> want to see "apply: corrupt patch at line 5".
>
> The reason for this error was that the hunk header looks like this:
>
> @@ -1 +1,2 @@
>
> but the existing code expects the original range always to contain a
> comma. This problem is easily fixed by cutting the string "1 +1,2"
> (that Git GUI formerly mistook for the starting line) at the space.
>
> This fixes https://github.com/git-for-windows/git/issues/515
>
> Signed-off-by: Johannes Schindelin
> ---
> lib/diff.tcl | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 4cae10a4c7..68c4a6c736 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -698,6 +698,7 @@ proc apply_range_or_line {x y} {
> set hh [$ui_diff get $i_l "$i_l + 1 lines"]
> set hh [lindex [split $hh ,] 0]
> set hln [lindex [split $hh -] 1]
> + set hln [lindex [split $hln " "] 0]
this is already in that master
https://github.com/prati0100/git-gui/blob/60c60b627e81bf84e1cb01729d2ae882178f079d/lib/diff.tcl#L727
>
> # There is a special situation to take care of. Consider this
> # hunk:
> --
> gitgitgadget
[PATCH 2/2] git-gui: support for diff3 conflict style
This adds highlight support for the diff3 conflict style.
The common pre-image will be reversed to --, because it has been removed
and either replaced with ours or theirs side.
Signed-off-by: Bert Wesarg
---
git-gui.sh | 3 +++
lib/diff.tcl | 22 ++
2 files changed, 25 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..6d80f82 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
$ui_diff tag conf d< \
-foreground orange \
-font font_diffbold
+$ui_diff tag conf d| \
+ -foreground orange \
+ -font font_diffbold
$ui_diff tag conf d= \
-foreground orange \
-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0fd4600..6caf4e7 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -347,6 +347,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
}
set ::current_diff_inheader 1
+ set ::conflict_state {CONTEXT}
fconfigure $fd \
-blocking 0 \
-encoding [get_path_encoding $path] \
@@ -450,10 +451,28 @@ proc read_diff {fd conflict_size cont_info} {
{++} {
set regexp [string map [list %conflict_size
$conflict_size]\
{^\+\+([<>=]){%conflict_size}(?: |$)}]
+ set regexp_pre_image [string map [list
%conflict_size $conflict_size]\
+
{^\+\+\|{%conflict_size}(?: |$)}]
if {[regexp $regexp $line _g op]} {
set is_conflict_diff 1
set line [string replace $line 0 1 { }]
+ set markup {}
set tags d$op
+ switch -exact -- $op {
+ < { set ::conflict_state {OURS} }
+ = { set ::conflict_state {THEIRS} }
+ > { set ::conflict_state {CONTEXT} }
+ }
+ } elseif {[regexp $regexp_pre_image $line]} {
+ set is_conflict_diff 1
+ set line [string replace $line 0 1 { }]
+ set markup {}
+ set tags d|
+ set ::conflict_state {BASE}
+ } elseif {$::conflict_state eq {BASE}} {
+ set line [string replace $line 0 1 {--}]
+ set markup {}
+ set tags d_--
} else {
set tags d_++
}
@@ -505,6 +524,9 @@ proc read_diff {fd conflict_size cont_info} {
}
}
set mark [$ui_diff index "end - 1 line linestart"]
+ if {[llength $markup] > 0} {
+ set tags {}
+ }
$ui_diff insert end $line $tags
if {[string index $line end] eq "\r"} {
$ui_diff tag add d_cr {end - 2c}
--
2.21.0.789.ga095d9d866
[PATCH 1/2] git-gui: use existing interface to query a path's attribute
Replace the hand-coded call to git check-attr with the already provided one.
Signed-off-by: Bert Wesarg
---
lib/diff.tcl | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 958a0fa..0fd4600 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
}
}
-proc get_conflict_marker_size {path} {
- set size 7
- catch {
- set fd_rc [eval [list git_read check-attr
"conflict-marker-size" -- $path]]
- set ret [gets $fd_rc line]
- close $fd_rc
- if {$ret > 0} {
- regexp {.*: conflict-marker-size: (\d+)$} $line line
size
- }
- }
- return $size
-}
-
proc start_show_diff {cont_info {add_opts {}}} {
global file_states file_lists
global is_3way_diff is_submodule_diff diff_active repo_config
@@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
set is_submodule_diff 0
set diff_active 1
set current_diff_header {}
- set conflict_size [get_conflict_marker_size $path]
+ set conflict_size [gitattr $path conflict-marker-size 7]
set cmd [list]
if {$w eq $ui_index} {
--
2.21.0.789.ga095d9d866
Re: git-gui: failure to distinguish 3-way common ancestors in hunk markers (#2340)
Pratyush, once again, I had done this years ago. I may post an updated patch in the evening: https://github.com/bertwesarg/git-gui/commit/90ee4a8c260132a6b730040095929fd267cedd7b Bert On Wed, Sep 25, 2019 at 12:05 AM Philip Oakley wrote: > > Hi list, > cc Pratyush, > > [resend without attached png file] > > While rebasing an old series, I had a 3-way merge fall back that didn't > show the `||| merged common ancestors` very well in git-gui. > > That is, the conflict markers, and common ancestor lines, are treated as > being part of the current HEAD hunk, rather than being separated. > > I opened a Git for Windows issue > https://github.com/git-for-windows/git/issues/2340 which has the > screenshot of the git-gui markers. > > I've not had any chance to look at the underlying code, but thought it > worth reporting. I guess the issue has been there a while. > > Philip > > >
Re: Git Gui - enhancement suggestion - Can a double click on the file name in the “unstaged” area move the item to “staged changes”
On Fri, Sep 13, 2019 at 4:32 PM Pratyush Yadav wrote: > > On 13/09/19 12:24PM, Allan Ford wrote: > > Dear Git Authors, > > > > Not a bug, but a suggestion consideration for “Git Gui” > > > > Can a double click on the file name in the “unstaged” area move the > > item to “staged changes” .. (rather than having to click on the small > > icon to the left of the file name?) > > It has been something on my radar for some time. Shouldn't be something > too difficult to do. > > While I like the idea in general, I have a question that I'd like to ask > other git-gui users: I miss a general problem description: Whats wrong with the single-click on the icon to begin with? I consider adding a second way as not not acceptable. I also consider double-click on a file in a GUI an "open" action. But in git-gui, this "open" action (showing the diff) is already done with a single-click. >From my point of view, it can stay as is. Best, Bert > > If we implement something like this, what happens when you single-click > on the icon? Do we treat that as a stage/unstage command? If we keep the > legacy behaviour of single-click on the icon stages/unstages, then a > part of the row is single-click and the rest double-click. > > If we make an entire row of the stage/unstage widget double click, it > messes with people who are already used to it. > > Is partial single and partial double click behaviour acceptable? Or > should we make the entire row double click only? Or something else that > I missed? > > -- > Regards, > Pratyush Yadav
Re: [PATCH] git-gui: convert new/amend commit radiobutton to checketton
On Fri, Sep 13, 2019 at 9:49 PM Pratyush Yadav wrote: > > You missed fixing the typo in the subject. s/checketton/checkbutton/. > But no need to send a re-roll for that. I can fix it locally. thanks, I also fixed that locally, but again, it slipped thru in the patch mail. Still don't know why this happened. Best, Bert > > Other than that, LGTM. Thanks, will queue. > > On 13/09/19 08:02AM, Bert Wesarg wrote: > > Its a bi-state anyway and also saves one line in the menu. > > > > Signed-off-by: Bert Wesarg > > -- > Regards, > Pratyush Yadav
[PATCH v4] git-gui: add horizontal scrollbar to commit buffer
While the commit message widget has a configurable fixed width, it
nevertheless allowed to write commit messages which exceeded this limit.
Though there is no visual clue, that there is scrolling going on. Now
there is a horizontal scrollbar.
There seems to be a bug in at least Tcl/Tk up to version 8.6.8, which
does not update the horizontal scrollbar if one removes the whole
content at once.
Suggested-by: Birger Skogeng Pedersen
Signed-off-by: Bert Wesarg
---
git-gui.sh | 6 ++
1 file changed, 6 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index 5bc21b8..ad962d4 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3363,10 +3363,16 @@ 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 .vpane.lower.commarea.buffer.header -side top -fill x
--
2.21.0.789.ga095d9d866
Re: [PATCH] git-gui: add horizontal scrollbar to commit buffer
On Fri, Sep 13, 2019 at 8:53 PM Pratyush Yadav wrote:
>
> Can you please add a version number when you send re-rolls the next time
> around. Something like "[PATCH v2] subject...". Makes it easier for me
> to keep track of things when there are multiple re-rolls of multiple
> patches.
>
> You can do this by passing "-v2" (or "-v3", "-v4", etc) to
> git-send-email or git-format-patch.
>
> You missed two quick questions I had in the last version. I'll ask them
> again below. Other than those two, LGTM. Thanks.
I removed both of then, but they still made it into the patch, sorry.
Will re-roll with v4 now.
Bert
>
> On 12/09/19 09:20PM, Bert Wesarg wrote:
> > While the commit message widget has a configurable fixed width, it
> > nevertheless allowed to write commit messages which exceeded this limit.
> > Though there is no visual clue, that there is scrolling going on. Now
> > there is a horizontal scrollbar.
>
> Looks much better!
>
> > There seems to be a bug in at least Tcl/Tk up to version 8.6.8, which
> > does not update the horizontal scrollbar if one removes the whole
> > content at once.
> >
> > Suggested-by: Birger Skogeng Pedersen
> > Signed-off-by: Bert Wesarg
> > ---
> > git-gui.sh | 10 --
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index 5bc21b8..032ebd6 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
>
> If I remove this change, the behavior does not seem to change, and the
> commit message buffer still expands. So what exactly does this change
> do?
>
> > 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 change the side from left to
> bottom? I tested by changing it back to left and it doesn't seem to make
> a difference.
>
> > pack .vpane.lower.commarea.buffer -side left -fill y
> >
> > # -- Commit Message Buffer Context Menu
> > --
> > 2.21.0.789.ga095d9d866
> >
>
> --
> Regards,
> Pratyush Yadav
[PATCH] git-gui: convert new/amend commit radiobutton to checketton
Its a bi-state anyway and also saves one line in the menu.
Signed-off-by: Bert Wesarg
---
git-gui.sh | 36 +---
lib/checkout_op.tcl | 6 +++---
lib/commit.tcl | 4 ++--
lib/index.tcl | 8
4 files changed, 18 insertions(+), 36 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 5bc21b8..80a07d5 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1341,6 +1341,7 @@ set HEAD {}
set PARENT {}
set MERGE_HEAD [list]
set commit_type {}
+set commit_type_is_amend 0
set empty_tree {}
set current_branch {}
set is_detached 0
@@ -1348,7 +1349,6 @@ set current_diff_path {}
set is_3way_diff 0
set is_submodule_diff 0
set is_conflict_diff 0
-set selected_commit_type new
set diff_empty_count 0
set nullid ""
@@ -1435,7 +1435,7 @@ proc PARENT {} {
}
proc force_amend {} {
- global selected_commit_type
+ global commit_type_is_amend
global HEAD PARENT MERGE_HEAD commit_type
repository_state newType newHEAD newMERGE_HEAD
@@ -1444,7 +1444,7 @@ proc force_amend {} {
set MERGE_HEAD $newMERGE_HEAD
set commit_type $newType
- set selected_commit_type amend
+ set commit_type_is_amend 1
do_select_commit_type
}
@@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled
singlecommit]} {
menu .mbar.commit
if {![is_enabled nocommit]} {
- .mbar.commit add radiobutton \
- -label [mc "New Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value new
- lappend disable_on_lock \
- [list .mbar.commit entryconf [.mbar.commit index last]
-state]
-
- .mbar.commit add radiobutton \
+ .mbar.commit add checkbutton \
-label [mc "Amend Last Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value amend
+ -variable commit_type_is_amend \
+ -command do_select_commit_type
lappend disable_on_lock \
[list .mbar.commit entryconf [.mbar.commit index last]
-state]
@@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t
set ui_coml .vpane.lower.commarea.buffer.header.l
if {![is_enabled nocommit]} {
- ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \
- -text [mc "New Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value new
- lappend disable_on_lock \
- [list .vpane.lower.commarea.buffer.header.new conf -state]
- ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \
+ ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \
-text [mc "Amend Last Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value amend
+ -variable commit_type_is_amend \
+ -command do_select_commit_type
lappend disable_on_lock \
[list .vpane.lower.commarea.buffer.header.amend conf -state]
}
@@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x
if {![is_enabled nocommit]} {
pack .vpane.lower.commarea.buffer.header.amend -side right
- pack .vpane.lower.commarea.buffer.header.new -side right
}
textframe .vpane.lower.commarea.buffer.frame
diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl
index 9e7412c..a522829 100644
--- a/lib/checkout_op.tcl
+++ b/lib/checkout_op.tcl
@@ -389,7 +389,7 @@ $err
}
method _after_readtree {} {
- global selected_commit_type commit_type HEAD MERGE_HEAD PARENT
+ global commit_type HEAD MERGE_HEAD PARENT
global current_branch is_detached
global ui_comm
@@ -490,12 +490,12 @@ method _update_repo_state {} {
#amend mode our file lists are accurate and we can avoid
#the rescan.
#
- global selected_commit_type commit_type HEAD MERGE_HEAD PARENT
+ global commit_type_is_amend commit_type HEAD MERGE_HEAD PARENT
global ui_comm
unlock_index
set name [_name $this]
- set selected_commit_type new
+ set commit_type_is_amend 0
if {[string match amend* $commit_type]} {
$ui_comm delete 0.0 end
$ui_comm edit reset
diff --git a/lib/commit.tcl b/lib/commit.tcl
index 83620b7..384f18f 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -327,7 +327,7 @@ proc commit_writetree {curHEAD msg_p} {
proc commit_committree {fd_wt curHEAD msg_p} {
global HEAD PARENT MERGE_HEAD commit_type commit_author
global current_
[PATCH] git-gui: convert new/amend commit radiobutton to checketton
Its a bi-state anyway and also safes one line in the menu.
Signed-off-by: Bert Wesarg
---
git-gui.sh | 36 +---
lib/checkout_op.tcl | 6 +++---
lib/commit.tcl | 4 ++--
lib/index.tcl | 8
4 files changed, 18 insertions(+), 36 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 5bc21b8..80a07d5 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1341,6 +1341,7 @@ set HEAD {}
set PARENT {}
set MERGE_HEAD [list]
set commit_type {}
+set commit_type_is_amend 0
set empty_tree {}
set current_branch {}
set is_detached 0
@@ -1348,7 +1349,6 @@ set current_diff_path {}
set is_3way_diff 0
set is_submodule_diff 0
set is_conflict_diff 0
-set selected_commit_type new
set diff_empty_count 0
set nullid ""
@@ -1435,7 +1435,7 @@ proc PARENT {} {
}
proc force_amend {} {
- global selected_commit_type
+ global commit_type_is_amend
global HEAD PARENT MERGE_HEAD commit_type
repository_state newType newHEAD newMERGE_HEAD
@@ -1444,7 +1444,7 @@ proc force_amend {} {
set MERGE_HEAD $newMERGE_HEAD
set commit_type $newType
- set selected_commit_type amend
+ set commit_type_is_amend 1
do_select_commit_type
}
@@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled
singlecommit]} {
menu .mbar.commit
if {![is_enabled nocommit]} {
- .mbar.commit add radiobutton \
- -label [mc "New Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value new
- lappend disable_on_lock \
- [list .mbar.commit entryconf [.mbar.commit index last]
-state]
-
- .mbar.commit add radiobutton \
+ .mbar.commit add checkbutton \
-label [mc "Amend Last Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value amend
+ -variable commit_type_is_amend \
+ -command do_select_commit_type
lappend disable_on_lock \
[list .mbar.commit entryconf [.mbar.commit index last]
-state]
@@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t
set ui_coml .vpane.lower.commarea.buffer.header.l
if {![is_enabled nocommit]} {
- ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \
- -text [mc "New Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value new
- lappend disable_on_lock \
- [list .vpane.lower.commarea.buffer.header.new conf -state]
- ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \
+ ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \
-text [mc "Amend Last Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value amend
+ -variable commit_type_is_amend \
+ -command do_select_commit_type
lappend disable_on_lock \
[list .vpane.lower.commarea.buffer.header.amend conf -state]
}
@@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x
if {![is_enabled nocommit]} {
pack .vpane.lower.commarea.buffer.header.amend -side right
- pack .vpane.lower.commarea.buffer.header.new -side right
}
textframe .vpane.lower.commarea.buffer.frame
diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl
index 9e7412c..a522829 100644
--- a/lib/checkout_op.tcl
+++ b/lib/checkout_op.tcl
@@ -389,7 +389,7 @@ $err
}
method _after_readtree {} {
- global selected_commit_type commit_type HEAD MERGE_HEAD PARENT
+ global commit_type HEAD MERGE_HEAD PARENT
global current_branch is_detached
global ui_comm
@@ -490,12 +490,12 @@ method _update_repo_state {} {
#amend mode our file lists are accurate and we can avoid
#the rescan.
#
- global selected_commit_type commit_type HEAD MERGE_HEAD PARENT
+ global commit_type_is_amend commit_type HEAD MERGE_HEAD PARENT
global ui_comm
unlock_index
set name [_name $this]
- set selected_commit_type new
+ set commit_type_is_amend 0
if {[string match amend* $commit_type]} {
$ui_comm delete 0.0 end
$ui_comm edit reset
diff --git a/lib/commit.tcl b/lib/commit.tcl
index 83620b7..384f18f 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -327,7 +327,7 @@ proc commit_writetree {curHEAD msg_p} {
proc commit_committree {fd_wt curHEAD msg_p} {
global HEAD PARENT MERGE_HEAD commit_type commit_author
global current_
Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton
On Wed, Sep 11, 2019 at 10:15 PM Pratyush Yadav wrote:
>
> Typo in the subject. s/checketton/checkbutton/\
Will re-roll and drop the actual keybinding patch, so that Birger can
resend his part,
Bert
>
> On 05/09/19 10:09PM, Bert Wesarg wrote:
> > Signed-off-by: Bert Wesarg
> > ---
> > git-gui.sh | 36 +---
> > lib/checkout_op.tcl | 6 +++---
> > lib/commit.tcl | 4 ++--
> > lib/index.tcl | 8
> > 4 files changed, 18 insertions(+), 36 deletions(-)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index 5bc21b8..80a07d5 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -1341,6 +1341,7 @@ set HEAD {}
> > set PARENT {}
> > set MERGE_HEAD [list]
> > set commit_type {}
> > +set commit_type_is_amend 0
> > set empty_tree {}
> > set current_branch {}
> > set is_detached 0
> > @@ -1348,7 +1349,6 @@ set current_diff_path {}
> > set is_3way_diff 0
> > set is_submodule_diff 0
> > set is_conflict_diff 0
> > -set selected_commit_type new
> > set diff_empty_count 0
> >
> > set nullid ""
> > @@ -1435,7 +1435,7 @@ proc PARENT {} {
> > }
> >
> > proc force_amend {} {
> > - global selected_commit_type
> > + global commit_type_is_amend
> > global HEAD PARENT MERGE_HEAD commit_type
> >
> > repository_state newType newHEAD newMERGE_HEAD
> > @@ -1444,7 +1444,7 @@ proc force_amend {} {
> > set MERGE_HEAD $newMERGE_HEAD
> > set commit_type $newType
> >
> > - set selected_commit_type amend
> > + set commit_type_is_amend 1
>
> Why do we need a separate variable for this? Why not just check
> commit_type to know whether it is an amend or not? My guess after
> reading the patch is that we need to associate a variable with the
> checkbutton to decide its state, and since there are multiple types of
> amend that commit_type can be (amend, amend-initial, amend-merge), it is
> not easy to use it directly. Am I guessing correctly?
>
> > do_select_commit_type
> > }
> >
> > @@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled
> > singlecommit]} {
> > menu .mbar.commit
> >
> > if {![is_enabled nocommit]} {
> > - .mbar.commit add radiobutton \
> > - -label [mc "New Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value new
> > - lappend disable_on_lock \
> > - [list .mbar.commit entryconf [.mbar.commit index
> > last] -state]
> > -
> > - .mbar.commit add radiobutton \
> > + .mbar.commit add checkbutton \
> > -label [mc "Amend Last Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value amend
> > + -variable commit_type_is_amend \
> > + -command do_select_commit_type
> > lappend disable_on_lock \
> > [list .mbar.commit entryconf [.mbar.commit index
> > last] -state]
> >
> > @@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t
> > set ui_coml .vpane.lower.commarea.buffer.header.l
> >
> > if {![is_enabled nocommit]} {
> > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \
> > - -text [mc "New Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value new
> > - lappend disable_on_lock \
> > - [list .vpane.lower.commarea.buffer.header.new conf -state]
> > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \
> > + ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \
> > -text [mc "Amend Last Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value amend
> > + -variable commit_type_is_amend \
> > + -command do_select_commit_type
> > lappend disable_on_lock \
> > [list .vpane.lower.commarea.buffer.header.amend conf -state]
> > }
> > @@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x
> &
Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton
On Wed, Sep 11, 2019 at 10:15 PM Pratyush Yadav wrote:
>
> Typo in the subject. s/checketton/checkbutton/
>
> On 05/09/19 10:09PM, Bert Wesarg wrote:
> > Signed-off-by: Bert Wesarg
> > ---
> > git-gui.sh | 36 +---
> > lib/checkout_op.tcl | 6 +++---
> > lib/commit.tcl | 4 ++--
> > lib/index.tcl | 8
> > 4 files changed, 18 insertions(+), 36 deletions(-)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index 5bc21b8..80a07d5 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -1341,6 +1341,7 @@ set HEAD {}
> > set PARENT {}
> > set MERGE_HEAD [list]
> > set commit_type {}
> > +set commit_type_is_amend 0
> > set empty_tree {}
> > set current_branch {}
> > set is_detached 0
> > @@ -1348,7 +1349,6 @@ set current_diff_path {}
> > set is_3way_diff 0
> > set is_submodule_diff 0
> > set is_conflict_diff 0
> > -set selected_commit_type new
> > set diff_empty_count 0
> >
> > set nullid ""
> > @@ -1435,7 +1435,7 @@ proc PARENT {} {
> > }
> >
> > proc force_amend {} {
> > - global selected_commit_type
> > + global commit_type_is_amend
> > global HEAD PARENT MERGE_HEAD commit_type
> >
> > repository_state newType newHEAD newMERGE_HEAD
> > @@ -1444,7 +1444,7 @@ proc force_amend {} {
> > set MERGE_HEAD $newMERGE_HEAD
> > set commit_type $newType
> >
> > - set selected_commit_type amend
> > + set commit_type_is_amend 1
>
> Why do we need a separate variable for this? Why not just check
> commit_type to know whether it is an amend or not? My guess after
> reading the patch is that we need to associate a variable with the
> checkbutton to decide its state, and since there are multiple types of
> amend that commit_type can be (amend, amend-initial, amend-merge), it is
> not easy to use it directly. Am I guessing correctly?
>
> > do_select_commit_type
> > }
> >
> > @@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled
> > singlecommit]} {
> > menu .mbar.commit
> >
> > if {![is_enabled nocommit]} {
> > - .mbar.commit add radiobutton \
> > - -label [mc "New Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value new
> > - lappend disable_on_lock \
> > - [list .mbar.commit entryconf [.mbar.commit index
> > last] -state]
> > -
> > - .mbar.commit add radiobutton \
> > + .mbar.commit add checkbutton \
> > -label [mc "Amend Last Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value amend
> > + -variable commit_type_is_amend \
> > + -command do_select_commit_type
> > lappend disable_on_lock \
> > [list .mbar.commit entryconf [.mbar.commit index
> > last] -state]
> >
> > @@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t
> > set ui_coml .vpane.lower.commarea.buffer.header.l
> >
> > if {![is_enabled nocommit]} {
> > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \
> > - -text [mc "New Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value new
> > - lappend disable_on_lock \
> > - [list .vpane.lower.commarea.buffer.header.new conf -state]
> > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \
> > + ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \
> > -text [mc "Amend Last Commit"] \
> > - -command do_select_commit_type \
> > - -variable selected_commit_type \
> > - -value amend
> > + -variable commit_type_is_amend \
> > + -command do_select_commit_type
> > lappend disable_on_lock \
> > [list .vpane.lower.commarea.buffer.header.amend conf -state]
> > }
> > @@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x
> >
> > if {![is_enabled nocommit]} {
> > pack .vpane.lower.commarea.buffer.header.amend
Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit
On Tue, Sep 10, 2019 at 6:50 PM Johannes Sixt 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
> > ---
> > 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
[PATCH] git-gui: add horizontal scrollbar to commit buffer
While the commit message widget has a configurable fixed width, it
nevertheless allowed to write commit messages which exceeded this limit.
Though there is no visual clue, that there is scrolling going on. Now
there is a horizontal scrollbar.
There seems to be a bug in at least Tcl/Tk up to version 8.6.8, which
does not update the horizontal scrollbar if one removes the whole
content at once.
Suggested-by: Birger Skogeng Pedersen
Signed-off-by: Bert Wesarg
---
git-gui.sh | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 5bc21b8..032ebd6 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 1/2] git-gui: warn if the commit message contains lines longer than the set limit
On Tue, Sep 10, 2019 at 10:35 PM Pratyush Yadav 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
Re: [PATCH 2/2] git-gui: add horizontal scrollbar to commit buffer
On Tue, Sep 10, 2019 at 10:28 PM Pratyush Yadav 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
> > Signed-off-by: Bert Wesarg
> > ---
> > 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 v3 0/4] git-gui: Add ability to revert selected hunks and lines
On Tue, Sep 10, 2019 at 10:26 PM Johannes Sixt wrote: > > Am 10.09.19 um 21:21 schrieb Pratyush Yadav: > > If there are no further objections with the series, I will merge it in. > > No objections. I use it in production. yep, Since 2012 ;-) > > -- Hannes
Re: [PATCH v2] git-gui: add hotkey to toggle "Amend Last Commit" radio selector
On Wed, Sep 4, 2019 at 8:00 PM Birger Skogeng Pedersen
wrote:
>
> Selecting whether to do a "New Commit" or "Amend Last Commit" does not have
> a hotkey.
>
> With this patch, the user may toggle between the two options with
> CTRL/CMD+e.
>
> Signed-off-by: Birger Skogeng Pedersen
> Signed-off-by: Bert Wesarg
> ---
> git-gui.sh | 40 +++-
> 1 file changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 5bc21b8..47c5db0 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1445,7 +1445,7 @@ proc force_amend {} {
> set commit_type $newType
>
> set selected_commit_type amend
> - do_select_commit_type
> + ui_select_commit_type
> }
>
> proc rescan {after {honor_trustmtime 1}} {
> @@ -2640,6 +2640,16 @@ proc show_less_context {} {
> }
> }
>
> +proc toggle_commit_type {} {
> + global selected_commit_type
> + if {[string match amend* $selected_commit_type]} {
only $commit_type has multiple incarnations of amend, thus this should
have been $commit_type here.
Though I posted new patches which convert it first to b a checkbutton
and than addedyour keybinding patch on top of it.
Bert
> + set selected_commit_type new
> + } else {
> + set selected_commit_type amend
> + }
> + ui_select_commit_type
> +}
> +
> ##
> ##
> ## ui construction
> @@ -2824,13 +2834,31 @@ proc commit_btn_caption {} {
> }
> }
>
> +proc ui_select_commit_type {} {
> + global selected_commit_type
> + global ui_commit_type_commit ui_commit_type_amend
> +
> + do_select_commit_type
> + if {$selected_commit_type eq {new}} {
> + .mbar.commit entryconf [mc "New Commit"] \
> + -accelerator {}
> + .mbar.commit entryconf [mc "Amend Last Commit"] \
> + -accelerator $::M1T-E
> + } elseif {$selected_commit_type eq {amend}} {
> + .mbar.commit entryconf [mc "New Commit"] \
> + -accelerator $::M1T-E
> + .mbar.commit entryconf [mc "Amend Last Commit"] \
> + -accelerator {}
> + }
> +}
> +
> if {[is_enabled multicommit] || [is_enabled singlecommit]} {
> menu .mbar.commit
>
> if {![is_enabled nocommit]} {
> .mbar.commit add radiobutton \
> -label [mc "New Commit"] \
> - -command do_select_commit_type \
> + -command ui_select_commit_type \
> -variable selected_commit_type \
> -value new
> lappend disable_on_lock \
> @@ -2838,7 +2866,8 @@ if {[is_enabled multicommit] || [is_enabled
> singlecommit]} {
>
> .mbar.commit add radiobutton \
> -label [mc "Amend Last Commit"] \
> - -command do_select_commit_type \
> + -accelerator $M1T-E \
> + -command ui_select_commit_type \
> -variable selected_commit_type \
> -value amend
> lappend disable_on_lock \
> @@ -3315,14 +3344,14 @@ set ui_coml .vpane.lower.commarea.buffer.header.l
> if {![is_enabled nocommit]} {
> ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \
> -text [mc "New Commit"] \
> - -command do_select_commit_type \
> + -command ui_select_commit_type \
> -variable selected_commit_type \
> -value new
> lappend disable_on_lock \
> [list .vpane.lower.commarea.buffer.header.new conf -state]
> ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \
> -text [mc "Amend Last Commit"] \
> - -command do_select_commit_type \
> + -command ui_select_commit_type \
> -variable selected_commit_type \
> -value amend
> lappend disable_on_lock \
> @@ -3843,6 +3872,7 @@ bind . <$M1B-Key-equal> {show_more_context;break}
> bind . <$M1B-Key-plus> {show_more_context;break}
> bind . <$M1B-Key-KP_Add> {show_more_context;break}
> bind . <$M1B-Key-Return> do_commit
> +bind . <$M1B-Key-e> toggle_commit_type
> foreach i [list $ui_index $ui_workdir] {
> bind $i{ toggle_or_diff click %W %x %y; break }
> bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break }
> --
> 2.21.0.windows.1
>
[PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu
From: Birger Skogeng Pedersen
Selecting whether to "Amend Last Commit" or not does not have a hotkey.
With this patch, the user may toggle between the two options with
CTRL/CMD+e.
Signed-off-by: Birger Skogeng Pedersen
Rebased-by: Bert Wesarg
---
git-gui.sh | 8
1 file changed, 8 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index 80a07d5..8b776dd 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2640,6 +2640,12 @@ proc show_less_context {} {
}
}
+proc toggle_commit_type {} {
+ global commit_type_is_amend
+ set commit_type_is_amend [expr !$commit_type_is_amend]
+ do_select_commit_type
+}
+
##
##
## ui construction
@@ -2830,6 +2836,7 @@ if {[is_enabled multicommit] || [is_enabled
singlecommit]} {
if {![is_enabled nocommit]} {
.mbar.commit add checkbutton \
-label [mc "Amend Last Commit"] \
+ -accelerator $M1T-E \
-variable commit_type_is_amend \
-command do_select_commit_type
lappend disable_on_lock \
@@ -3825,6 +3832,7 @@ bind . <$M1B-Key-equal> {show_more_context;break}
bind . <$M1B-Key-plus> {show_more_context;break}
bind . <$M1B-Key-KP_Add> {show_more_context;break}
bind . <$M1B-Key-Return> do_commit
+bind . <$M1B-Key-e> toggle_commit_type
foreach i [list $ui_index $ui_workdir] {
bind $i{ toggle_or_diff click %W %x %y; break }
bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break }
--
2.21.0.789.ga095d9d866
[PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton
Signed-off-by: Bert Wesarg
---
git-gui.sh | 36 +---
lib/checkout_op.tcl | 6 +++---
lib/commit.tcl | 4 ++--
lib/index.tcl | 8
4 files changed, 18 insertions(+), 36 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 5bc21b8..80a07d5 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1341,6 +1341,7 @@ set HEAD {}
set PARENT {}
set MERGE_HEAD [list]
set commit_type {}
+set commit_type_is_amend 0
set empty_tree {}
set current_branch {}
set is_detached 0
@@ -1348,7 +1349,6 @@ set current_diff_path {}
set is_3way_diff 0
set is_submodule_diff 0
set is_conflict_diff 0
-set selected_commit_type new
set diff_empty_count 0
set nullid ""
@@ -1435,7 +1435,7 @@ proc PARENT {} {
}
proc force_amend {} {
- global selected_commit_type
+ global commit_type_is_amend
global HEAD PARENT MERGE_HEAD commit_type
repository_state newType newHEAD newMERGE_HEAD
@@ -1444,7 +1444,7 @@ proc force_amend {} {
set MERGE_HEAD $newMERGE_HEAD
set commit_type $newType
- set selected_commit_type amend
+ set commit_type_is_amend 1
do_select_commit_type
}
@@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled
singlecommit]} {
menu .mbar.commit
if {![is_enabled nocommit]} {
- .mbar.commit add radiobutton \
- -label [mc "New Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value new
- lappend disable_on_lock \
- [list .mbar.commit entryconf [.mbar.commit index last]
-state]
-
- .mbar.commit add radiobutton \
+ .mbar.commit add checkbutton \
-label [mc "Amend Last Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value amend
+ -variable commit_type_is_amend \
+ -command do_select_commit_type
lappend disable_on_lock \
[list .mbar.commit entryconf [.mbar.commit index last]
-state]
@@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t
set ui_coml .vpane.lower.commarea.buffer.header.l
if {![is_enabled nocommit]} {
- ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \
- -text [mc "New Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value new
- lappend disable_on_lock \
- [list .vpane.lower.commarea.buffer.header.new conf -state]
- ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \
+ ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \
-text [mc "Amend Last Commit"] \
- -command do_select_commit_type \
- -variable selected_commit_type \
- -value amend
+ -variable commit_type_is_amend \
+ -command do_select_commit_type
lappend disable_on_lock \
[list .vpane.lower.commarea.buffer.header.amend conf -state]
}
@@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x
if {![is_enabled nocommit]} {
pack .vpane.lower.commarea.buffer.header.amend -side right
- pack .vpane.lower.commarea.buffer.header.new -side right
}
textframe .vpane.lower.commarea.buffer.frame
diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl
index 9e7412c..a522829 100644
--- a/lib/checkout_op.tcl
+++ b/lib/checkout_op.tcl
@@ -389,7 +389,7 @@ $err
}
method _after_readtree {} {
- global selected_commit_type commit_type HEAD MERGE_HEAD PARENT
+ global commit_type HEAD MERGE_HEAD PARENT
global current_branch is_detached
global ui_comm
@@ -490,12 +490,12 @@ method _update_repo_state {} {
#amend mode our file lists are accurate and we can avoid
#the rescan.
#
- global selected_commit_type commit_type HEAD MERGE_HEAD PARENT
+ global commit_type_is_amend commit_type HEAD MERGE_HEAD PARENT
global ui_comm
unlock_index
set name [_name $this]
- set selected_commit_type new
+ set commit_type_is_amend 0
if {[string match amend* $commit_type]} {
$ui_comm delete 0.0 end
$ui_comm edit reset
diff --git a/lib/commit.tcl b/lib/commit.tcl
index 83620b7..384f18f 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -327,7 +327,7 @@ proc commit_writetree {curHEAD msg_p} {
proc commit_committree {fd_wt curHEAD msg_p} {
global HEAD PARENT MERGE_HEAD commit_type commit_author
global current_branch
- global ui_comm selected_commit_type
+ global ui_co
Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit
On Thu, Sep 5, 2019 at 8:50 AM Birger Skogeng Pedersen wrote: > > On Thu, Sep 5, 2019 at 12:48 AM Pratyush Yadav 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
On Thu, Sep 5, 2019 at 8:22 AM Johannes Sixt 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 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 2/2] git-gui: add horizontal scrollbar to commit buffer
On Wed, Sep 4, 2019 at 10:31 PM Eric Sunshine wrote: > > On Wed, Sep 4, 2019 at 4:10 PM 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 > > 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 > > Signed-off-by: Bert Wesarg
Re: [PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit
On Wed, Sep 4, 2019 at 10:30 PM Eric Sunshine wrote:
>
> On Wed, Sep 4, 2019 at 4:10 PM Bert Wesarg 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
> > ---
> > 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)?
[PATCH 1/2] git-gui: warn if the commit message contains lines longer than the set limit
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
---
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
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
Signed-off-by: Bert Wesarg
---
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 v5] git-gui: Add hotkeys to set widget focus
Dear Hannes, On Wed, Sep 4, 2019 at 8:59 PM Johannes Sixt wrote: > > Am 04.09.19 um 16:30 schrieb Birger Skogeng Pedersen: > > The user cannot change focus between the list of files, the diff view and > > the commit message widgets without using the mouse (clicking either of > > the four widgets). > > > > With this patch, the user may set ui focus to the previously selected path > > in either the "Unstaged Changes" or "Staged Changes" widgets, using > > ALT+1 or ALT+2. > > > > The user may also set the ui focus to the diff view widget with > > ALT+3, or to the commit message widget with ALT+4. > > Many keyboards do not have a right Alt-key. That means that Alt+1 to > Alt+4 combinations must be typed single-handed with the left hand. This > is mildly awkward for Alt+4. Can we please have the very important > commit widget *not* at Alt+4? I could live with Alt+3. > I use my left thumb to press the left Alt key and it does not feel mildly awkward. As Alt is also used for the mnemonics, there will probably more of mildly awkward key combinations, wont there? Bert > -- Hannes
Re: git-gui: Long lines in commit message gets hidden, no scrollbar appears
On Tue, Sep 3, 2019 at 7:15 PM Pratyush Yadav wrote: > > On 02/09/19 09:13PM, Bert Wesarg wrote: > > On Mon, Sep 2, 2019 at 9:03 PM Bert Wesarg > > wrote: > > > > [snip] > > > > On second thought, wouldn't it make more sense to expand the > > > > commit > > > > message buffer instead? The point of resizing that pane is to see more > > > > of the commit message. So it makes more sense to make the commit message > > > > buffer take up all the vertical space, rather than making the scrollbar > > > > move. > > > > > > it is, I just broke that ;-) > > > > is fixed in GitHub: > > > > wget > > https://github.com/bertwesarg/git-gui/commit/56163547604f44688e208393f8941efaf5247d40.patch > > I tried the patch out. Works fine on Linux. Thanks. > > There is a minor typo in your commit message. > > > Sugestted-by: Birger Skogeng Pedersen > > s/Sugestted/Suggested/ > > > Signed-off-by: Bert Wesarg > > One more observation: > > If I write a particularly long line (and consequently the scrollbar > becomes active), and then hit Ctrl+A to select all text, and then > backspace to delete it all, the scrollbar does not get updated. It still > shows the old position where is is "scrolled" halfway through. As soon > as I type in any other character, it takes the correct state, and > becomes disabled. > > The vertical scrollbar behaves correctly in this scenario, and does take > the correct state and position as soon as I delete all text, so I > suspect it should be a small fix. Maybe a missed option or something > like that? While I can reproduce this, I don't figured out what is wrong here. I tried a minimal example and this also fails. The yScrollCommand is issued, but the xScrollCommand not. I have tcl/tk 8.6.8 on Linux. Bert > > -- > Regards, > Pratyush Yadav
Re: feature request, git-gui: add hotkey to toggle amend/new
On Wed, Sep 4, 2019 at 8:52 PM Johannes Sixt wrote: > > Am 04.09.19 um 19:46 schrieb Pratyush Yadav: > > On 04/09/19 08:24AM, Johannes Sixt wrote: > >> That is worth a try. The check box title offers a natural hotkey then: > >> "_A_mend last commit", Alt-a. > > > > Right now, the binding proposed is Ctrl-e. My mental model for the key > > bindings as of now is having the "actions" bound to Ctrl, and bindings > > that move you around in the UI bound to Alt. So it makes more sense to > > me to have a "amend toggle" bound to Ctrl. Maybe that's just me though. > > Anyone else care to chime in? > > "Amend last commit" is NOT an action. It switches a state. > > It is common in Windows GUIs that every control, including menu items, > has a hotkey associated, the underlined letter in the caption, and the > hotkey to access that UI control is Alt+that letter. It's not > necessarily a matter of moving around. > > And, BTW, this hotkey thing is also the case on my Linux desktop > (KDE-based). > > But of course, git-gui is different and totally off track here. It has > *zero* controls marked for hotkey-accessibility. I was just hoping to > spark an effort to make some of the controls marked and hotkey-accessible. I'm in favor of making this a checkbox, also in the menu. As all menu entries have currently a CTRL+ binding assigned, I think this one should have one too. As CTRL+A is taken, and the proposal is CTRL+E, I would be fine with this. If the menus and the UX elements also honors mnemonic now or in the future, I don't think they need to match the CTRL+ binding. Thus if this gets Alt+A I'm fine with this too. Bert > > -- Hannes
Re: [PATCH] Add hotkey to toggle "Amend Last Commit" radio selector
On Wed, Sep 4, 2019 at 7:36 PM Pratyush Yadav wrote: > > On 04/09/19 07:58AM, Birger Skogeng Pedersen wrote: > > Hi Pratyush, > > > > > > So how does this work? Should I email the patch that Bert has created? > > Or is it okay that it just remains on Github. (Considering the git > > mail archives) > > I think it would be a good idea to send it on the list once before we go > ahead with it. This way other people who are interested can take a > look. They might not necessarily pay enough attention to the threads to > notice the GitHub link. And there is, as you mentioned, the added > benefit of having it on the mailing list archives. I'm finally able to send patches around again. Though I find the idea from Johannes appealing to convert this to a check button. Are you with this? Bert > > -- > Regards, > Pratyush Yadav
Re: [PATCH] Add hotkey to toggle "Amend Last Commit" radio selector
Birger,
On Tue, Sep 3, 2019 at 10:52 AM Bert Wesarg wrote:
>
> Birger,
>
> On Mon, Sep 2, 2019 at 9:56 PM Birger Skogeng Pedersen
> wrote:
> >
> > Selecting whether to do a "New Commit" or "Amend Last Commit" does not have
> > a hotkey.
> >
> > With this patch, the user may toggle between the two options with
> > CTRL/CMD+e.
>
> David A. (in Cc from git-cola) suggested, that we try to re-use
> existing key bindings in Git GUIs. Here is Git-Cola hotkeys:
>
> http://git-cola.github.io/share/doc/git-cola/hotkeys.html
>
> and this indicates, that it already uses CTRL+m for "amend". Though
> I'm unsure if this is a toggle like in git-gui or a one-shot. David
> A., can you please clarify?
>
> >
> > Signed-off-by: Birger Skogeng Pedersen
> > ---
> > git-gui.sh | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index 5bc21b8..14be1e0 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -2640,6 +2640,16 @@ proc show_less_context {} {
> > }
> > }
> >
> > +proc toggle_commit_type {} {
> > + global selected_commit_type
> > + if {[string match amend* $selected_commit_type]} {
> > + set selected_commit_type new
> > + } else {
> > + set selected_commit_type amend
> > + }
> > + do_select_commit_type
> > +}
> > +
> > ##
> > ##
> > ## ui construction
> > @@ -3843,6 +3853,7 @@ bind . <$M1B-Key-equal> {show_more_context;break}
> > bind . <$M1B-Key-plus> {show_more_context;break}
> > bind . <$M1B-Key-KP_Add> {show_more_context;break}
> > bind . <$M1B-Key-Return> do_commit
> > +bind . <$M1B-Key-e> toggle_commit_type
>
> The commit type has also a two toggle menu entries (under "Commit")
> they should now also indicate the key binding.
> disable
>
> Though how to express a toggle keybinding in the menu? I don't know if
> you can assign the same keybinding to the same menu entry. Maybe we
> need to add/remove the keybinding depending on the current mode.
that works. I squashed this into your commit and pushed to GitHub
(sorry, still no path-per-mail)
https://github.com/bertwesarg/git-gui/commit/245bb9944e3d1c6b266c58b56d316f283ed3516b
Bert
>
> Bert
>
> > foreach i [list $ui_index $ui_workdir] {
> > bind $i{ toggle_or_diff click %W %x %y; break }
> > bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break }
> > --
> > 2.21.0.windows.1
> >
Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus
On Tue, Sep 3, 2019 at 4:22 PM Pratyush Yadav wrote:
>
> On 02/09/19 09:42PM, Bert Wesarg wrote:
> > On Sun, Sep 1, 2019 at 9:37 PM Birger Skogeng Pedersen
> > wrote:
> > >
> > > The user cannot change focus between the list of files, the diff view and
> > > the commit message widgets without using the mouse (clicking either of
> > > the four widgets).
> > >
> > > With this patch, the user may set ui focus to the previously selected path
> > > in either the "Unstaged Changes" or "Staged Changes" widgets, using
> > > CTRL/CMD+1 or CTRL/CMD+2.
> > >
> > > The user may also set the ui focus to the diff view widget with
> > > CTRL/CMD+3, or to the commit message widget with CTRL/CMD+4.
> > >
> > > This enables the user to select/unselect files, view the diff and create a
> > > commit in git-gui using keyboard-only.
> > >
> > > Signed-off-by: Birger Skogeng Pedersen
> > > ---
> > > git-gui.sh | 35 ++-
> > > 1 file changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/git-gui.sh b/git-gui.sh
> > > index 5bc21b8..ce620f1 100755
> > > --- a/git-gui.sh
> > > +++ b/git-gui.sh
> > > @@ -2495,7 +2495,7 @@ proc force_first_diff {after} {
> > >
> > > proc toggle_or_diff {mode w args} {
> > > global file_states file_lists current_diff_path ui_index
> > > ui_workdir
> > > - global last_clicked selected_paths
> > > + global last_clicked selected_paths file_lists_last_clicked
> > >
> > > if {$mode eq "click"} {
> > > foreach {x y} $args break
> > > @@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {
> > > $ui_index tag remove in_sel 0.0 end
> > > $ui_workdir tag remove in_sel 0.0 end
> > >
> > > + set file_lists_last_clicked($w) $lno
> >
> > So we only remember the lno in the widget, that could mean, that we
> > select the wrong file after a rescan, which shifted the previous path
> > one down. Can we remember the pathname instead, and try to find this
> > again in the file list?
>
> I raised a similar concern. Birger's response was that it is not a
> trivial change for him, and he needs help with it. So I decided to keep
> it like it is.
>
> But now I thought about it a bit more, and I don't think it should be
> too difficult. A quick look tells me that $file_lists should be a sorted
> list with unique entries (git-gui.sh::display_file_helper{}), so it
> shouldn't be too difficult to find a given path. display_file_helper
> does:
>
> set lno [lsearch -sorted -exact $file_lists($w) $path]
>
> which should also be what we want.
>
> I have a quick-and-dirty fix for it. Haven't tested it too well, but it
> seems to work for some basic things like removing a file and refreshing,
> and then selecting focus again. Do give it a spin. I'll send the patch
> in reply to this email.
>
> > > +
> > > # Determine the state of the file
> > > if {[info exists file_states($path)]} {
> > > set state [lindex $file_states($path) 0]
> > > @@ -2640,6 +2642,29 @@ proc show_less_context {} {
> > > }
> > > }
> > >
> > > +proc select_path_in {widget} {
> >
> > can we name it 'focus_and_select_path_in', as the main job ob this
> > function is to focus the widget. It makes also the 'bind' command
> > below more readily, because than all bind commands start with 'focus'.
>
> Ah, I was kind of uneasy with the function name, but couldn't come up
> with a good one. This sounds all right. Another suggestion that I'd put
> out: "focus_widget". As you said, focusing a widget is the main job of
> this function. Selecting paths is secondary. IMO it should be fine to
> not have "select_path_in" in the function name because you select the
> path in the process of focusing on widget.
>
> > > + global file_lists last_clicked selected_paths ui_workdir
> >
> > ui_workdir not referenced in this function
>
> Nice catch.
>
> > > + global file_lists_last_clicked
> > > +
> > > + set _list_length [llength $file_lists($widget)]
> > > + if {$_list_length > 0} {
> > > +
> > > + set _
Re: [PATCH] Add hotkey to toggle "Amend Last Commit" radio selector
David,
On Tue, Sep 3, 2019 at 10:52 AM Bert Wesarg wrote:
>
> Birger,
>
> On Mon, Sep 2, 2019 at 9:56 PM Birger Skogeng Pedersen
> wrote:
> >
> > Selecting whether to do a "New Commit" or "Amend Last Commit" does not have
> > a hotkey.
> >
> > With this patch, the user may toggle between the two options with
> > CTRL/CMD+e.
>
> David A. (in Cc from git-cola) suggested, that we try to re-use
> existing key bindings in Git GUIs. Here is Git-Cola hotkeys:
>
> http://git-cola.github.io/share/doc/git-cola/hotkeys.html
>
> and this indicates, that it already uses CTRL+m for "amend". Though
> I'm unsure if this is a toggle like in git-gui or a one-shot. David
> A., can you please clarify?
git-gui already assigns CTRL+m to "Local merge…"
sorry. I think trying to synchronize the keybindings seems impossible.
Bert
>
> >
> > Signed-off-by: Birger Skogeng Pedersen
> > ---
> > git-gui.sh | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index 5bc21b8..14be1e0 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -2640,6 +2640,16 @@ proc show_less_context {} {
> > }
> > }
> >
> > +proc toggle_commit_type {} {
> > + global selected_commit_type
> > + if {[string match amend* $selected_commit_type]} {
> > + set selected_commit_type new
> > + } else {
> > + set selected_commit_type amend
> > + }
> > + do_select_commit_type
> > +}
> > +
> > ##
> > ##
> > ## ui construction
> > @@ -3843,6 +3853,7 @@ bind . <$M1B-Key-equal> {show_more_context;break}
> > bind . <$M1B-Key-plus> {show_more_context;break}
> > bind . <$M1B-Key-KP_Add> {show_more_context;break}
> > bind . <$M1B-Key-Return> do_commit
> > +bind . <$M1B-Key-e> toggle_commit_type
>
> The commit type has also a two toggle menu entries (under "Commit")
> they should now also indicate the key binding.
> disable
>
> Though how to express a toggle keybinding in the menu? I don't know if
> you can assign the same keybinding to the same menu entry. Maybe we
> need to add/remove the keybinding depending on the current mode.
>
> Bert
>
> > foreach i [list $ui_index $ui_workdir] {
> > bind $i{ toggle_or_diff click %W %x %y; break }
> > bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break }
> > --
> > 2.21.0.windows.1
> >
Re: [PATCH] Add hotkey to toggle "Amend Last Commit" radio selector
Birger,
On Mon, Sep 2, 2019 at 9:56 PM Birger Skogeng Pedersen
wrote:
>
> Selecting whether to do a "New Commit" or "Amend Last Commit" does not have
> a hotkey.
>
> With this patch, the user may toggle between the two options with
> CTRL/CMD+e.
David A. (in Cc from git-cola) suggested, that we try to re-use
existing key bindings in Git GUIs. Here is Git-Cola hotkeys:
http://git-cola.github.io/share/doc/git-cola/hotkeys.html
and this indicates, that it already uses CTRL+m for "amend". Though
I'm unsure if this is a toggle like in git-gui or a one-shot. David
A., can you please clarify?
>
> Signed-off-by: Birger Skogeng Pedersen
> ---
> git-gui.sh | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 5bc21b8..14be1e0 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2640,6 +2640,16 @@ proc show_less_context {} {
> }
> }
>
> +proc toggle_commit_type {} {
> + global selected_commit_type
> + if {[string match amend* $selected_commit_type]} {
> + set selected_commit_type new
> + } else {
> + set selected_commit_type amend
> + }
> + do_select_commit_type
> +}
> +
> ##
> ##
> ## ui construction
> @@ -3843,6 +3853,7 @@ bind . <$M1B-Key-equal> {show_more_context;break}
> bind . <$M1B-Key-plus> {show_more_context;break}
> bind . <$M1B-Key-KP_Add> {show_more_context;break}
> bind . <$M1B-Key-Return> do_commit
> +bind . <$M1B-Key-e> toggle_commit_type
The commit type has also a two toggle menu entries (under "Commit")
they should now also indicate the key binding.
disable
Though how to express a toggle keybinding in the menu? I don't know if
you can assign the same keybinding to the same menu entry. Maybe we
need to add/remove the keybinding depending on the current mode.
Bert
> foreach i [list $ui_index $ui_workdir] {
> bind $i{ toggle_or_diff click %W %x %y; break }
> bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break }
> --
> 2.21.0.windows.1
>
Re: feature request, git-gui: add hotkey to toggle amend/new
David, On Tue, Sep 3, 2019 at 3:01 AM David wrote: > > On Tue, 3 Sep 2019 at 04:11, Bert Wesarg wrote: > > On Mon, Sep 2, 2019 at 6:25 PM Birger Skogeng Pedersen > > wrote: > > > On Sat, Aug 31, 2019 at 12:51 PM Birger Skogeng Pedersen > > > wrote: > > > > > In my pursuit to fully utilize git-gui with only using a keyboard, I > > > > suggest that there is a hotkey to toggle between selecting "New > > > > Commit" and "Amend Last Commit". > > Hi, thanks for maintaining and contributing to git and git-gui, it's a > great tool! > > > After focusing the commit message widget, you can focus the radio > > buttons with Tab/Shift+Tab and press Space. > > > I think this is short enough, so that wasting a Letter is not > > justified here. > > Ugh, may I express how unhappy I am to read that opinion from > the maintainer. I strongly disagree, please reconsider :( > > And I enthusiastically support this initial request for a single > hotkey to immediately toggle between "New Commit" and "Amend Last > Commit". And it should work regardless of wherever the cursor or > highlight is currently active. > > I have used git-gui for many years and I find this is actually the > most annoying and inconsistent aspect of its user interface. Sometimes > if one is lucky then "spacebar" will achieve it at startup or after > refresh, sometimes not. When I test here just now the suggested > tab/shift-tab/spacebar method, it does toggle but it also changes the > items in the staged changes list as an unwanted side effect. My > version says 0.20.0.8.gd000, but I have a few local patches (written > years ago) so sorry I am not testing with a version that you have, but > even so I wanted to report what I observed. > > If one is often amending commit messages as I do during large > interactive rebases, it is painful to have to do some kind of > context-sensitive multi-key dance just to change from "New Commit" to > "Amend Last Commit". Especially when every other operation has become > a single keystroke in my muscle memory. > > In my world it most definitely would not be "wasting a letter" to > implement this! It would instead be "OMG at last that got fixed for > everyone, hooray!" :D thanks for the input. Though I'm not the maintainer. Bert
Re: feature request, git-gui: add hotkey to toggle amend/new
On Mon, Sep 2, 2019 at 10:12 PM Bert Wesarg wrote: > > On Mon, Sep 2, 2019 at 9:49 PM Birger Skogeng Pedersen > wrote: > > > > Hi Bert, > > > > > > On Mon, Sep 2, 2019 at 8:08 PM Bert Wesarg > > wrote: > > > I think with your "focus" patch, this is not needed anymore: > > > > > > After focusing the commit message widget, you can focus the radio > > > buttons with Tab/Shift+Tab and press Space. > > > > > > I think this is short enough, so that wasting a Letter is not justified > > > here. > > > > Pressing the Tab key while the commit message widget is focused > > inserts a tab in the commit message. does Control-Tab works for traversal? > > (Again, I'm on Windows so you might get different behaviour on Linux) > > > > If the Tab key acted like you suggested, I agree it would not be > > necessary with this a hotkey like this. > > can we try to figure this out, before going forward with anything else? > > Thanks. > > Bert > > > > > > > Best regards, > > Birger
Re: feature request, git-gui: add hotkey to toggle amend/new
On Mon, Sep 2, 2019 at 9:49 PM Birger Skogeng Pedersen wrote: > > Hi Bert, > > > On Mon, Sep 2, 2019 at 8:08 PM Bert Wesarg wrote: > > I think with your "focus" patch, this is not needed anymore: > > > > After focusing the commit message widget, you can focus the radio > > buttons with Tab/Shift+Tab and press Space. > > > > I think this is short enough, so that wasting a Letter is not justified > > here. > > Pressing the Tab key while the commit message widget is focused > inserts a tab in the commit message. > (Again, I'm on Windows so you might get different behaviour on Linux) > > If the Tab key acted like you suggested, I agree it would not be > necessary with this a hotkey like this. can we try to figure this out, before going forward with anything else? Thanks. Bert > > > Best regards, > Birger
Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus
On Sun, Sep 1, 2019 at 9:37 PM Birger Skogeng Pedersen
wrote:
>
> The user cannot change focus between the list of files, the diff view and
> the commit message widgets without using the mouse (clicking either of
> the four widgets).
>
> With this patch, the user may set ui focus to the previously selected path
> in either the "Unstaged Changes" or "Staged Changes" widgets, using
> CTRL/CMD+1 or CTRL/CMD+2.
>
> The user may also set the ui focus to the diff view widget with
> CTRL/CMD+3, or to the commit message widget with CTRL/CMD+4.
>
> This enables the user to select/unselect files, view the diff and create a
> commit in git-gui using keyboard-only.
>
> Signed-off-by: Birger Skogeng Pedersen
> ---
> git-gui.sh | 35 ++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 5bc21b8..ce620f1 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2495,7 +2495,7 @@ proc force_first_diff {after} {
>
> proc toggle_or_diff {mode w args} {
> global file_states file_lists current_diff_path ui_index ui_workdir
> - global last_clicked selected_paths
> + global last_clicked selected_paths file_lists_last_clicked
>
> if {$mode eq "click"} {
> foreach {x y} $args break
> @@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {
> $ui_index tag remove in_sel 0.0 end
> $ui_workdir tag remove in_sel 0.0 end
>
> + set file_lists_last_clicked($w) $lno
So we only remember the lno in the widget, that could mean, that we
select the wrong file after a rescan, which shifted the previous path
one down. Can we remember the pathname instead, and try to find this
again in the file list?
> +
> # Determine the state of the file
> if {[info exists file_states($path)]} {
> set state [lindex $file_states($path) 0]
> @@ -2640,6 +2642,29 @@ proc show_less_context {} {
> }
> }
>
> +proc select_path_in {widget} {
can we name it 'focus_and_select_path_in', as the main job ob this
function is to focus the widget. It makes also the 'bind' command
below more readily, because than all bind commands start with 'focus'.
> + global file_lists last_clicked selected_paths ui_workdir
ui_workdir not referenced in this function
> + global file_lists_last_clicked
> +
> + set _list_length [llength $file_lists($widget)]
> + if {$_list_length > 0} {
> +
> + set _index $file_lists_last_clicked($widget)
I have the impression that variables starting with '_' are mainly used
as read-only global variables, see the list at line 158, and not that
often as temporal local variables.
> + if {$_index eq {}} {
> + set _index 1
> + } elseif {$_index > $_list_length} {
> + set _index $_list_length
> + }
> +
> + focus $widget
> + set last_clicked [list $widget $_index]
> + set path [lindex $file_lists($widget) [expr $_index - 1]]
> + array unset selected_paths
> + set selected_paths($path) 1
> + show_diff $path $widget
> + }
> +}
> +
> ##
> ##
> ## ui construction
> @@ -3852,6 +3877,14 @@ foreach i [list $ui_index $ui_workdir] {
> }
> unset i
>
> +bind . <$M1B-Key-1> {select_path_in $::ui_workdir}
> +bind . <$M1B-Key-2> {select_path_in $::ui_index}
> +bind . <$M1B-Key-3> {focus $::ui_diff}
> +bind . <$M1B-Key-4> {focus $::ui_comm}
I would like to bring up a proposal: AFAICS, more or less all CTRL
bindings have a menu entry. But it does not make sense to have a menu
entry for these bindings. And I think we could add more bindings for
keyboard-afine users. Thus I would like to propose to use ALT as the
modifier for these bindings, which would give us a nice binding
classification.
How about that?
Bert
> +
> +set file_lists_last_clicked($ui_index) {}
> +set file_lists_last_clicked($ui_workdir) {}
> +
> set file_lists($ui_index) [list]
> set file_lists($ui_workdir) [list]
>
> --
> 2.23.0.37.g745f681289
>
Re: git-gui: Long lines in commit message gets hidden, no scrollbar appears
On Mon, Sep 2, 2019 at 9:03 PM Bert Wesarg wrote: > > On Mon, Sep 2, 2019 at 8:58 PM Pratyush Yadav wrote: > > > > On 03/09/19 12:14AM, Pratyush Yadav wrote: > > > On 02/09/19 08:22PM, Birger Skogeng Pedersen wrote: > > > > On Mon, Sep 2, 2019 at 8:05 PM Bert Wesarg > > > > wrote: > > > > > I cannot test windows easily, it looks good on Linux Tcl /Tk 8.6: > > > > > > > > > > https://kgab.selfhost.eu/s/f38GX4caCZBj4mZ > > > > > > > > On Mon, Sep 2, 2019 at 8:12 PM Pratyush Yadav > > > > wrote: > > > > > Hmm, it looks fine for me. Which platform are you using? I am running > > > > > it > > > > > on Linux. Screenshot: https://imgur.com/sNp5Ktq > > > > > > > > Try resizing the bottom right pane of git gui, you should see that the > > > > scrollbar remains at the bottom while the input area moves upwards. > > > > > > Yes, I can reproduce the problem when I do this. Interestingly, the > > > vertical scrollbar does move, the horizontal one (which Bert just added) > > > doesn't. So I think there is a slight difference in how the horizontal > > > scrollbar is set up that is causing this. > > > > On second thought, wouldn't it make more sense to expand the commit > > message buffer instead? The point of resizing that pane is to see more > > of the commit message. So it makes more sense to make the commit message > > buffer take up all the vertical space, rather than making the scrollbar > > move. > > it is, I just broke that ;-) is fixed in GitHub: wget https://github.com/bertwesarg/git-gui/commit/56163547604f44688e208393f8941efaf5247d40.patch Thanks. Bert > > Bert > > > > > -- > > Regards, > > Pratyush Yadav
Re: git-gui: Long lines in commit message gets hidden, no scrollbar appears
On Mon, Sep 2, 2019 at 8:58 PM Pratyush Yadav wrote: > > On 03/09/19 12:14AM, Pratyush Yadav wrote: > > On 02/09/19 08:22PM, Birger Skogeng Pedersen wrote: > > > On Mon, Sep 2, 2019 at 8:05 PM Bert Wesarg > > > wrote: > > > > I cannot test windows easily, it looks good on Linux Tcl /Tk 8.6: > > > > > > > > https://kgab.selfhost.eu/s/f38GX4caCZBj4mZ > > > > > > On Mon, Sep 2, 2019 at 8:12 PM Pratyush Yadav > > > wrote: > > > > Hmm, it looks fine for me. Which platform are you using? I am running it > > > > on Linux. Screenshot: https://imgur.com/sNp5Ktq > > > > > > Try resizing the bottom right pane of git gui, you should see that the > > > scrollbar remains at the bottom while the input area moves upwards. > > > > Yes, I can reproduce the problem when I do this. Interestingly, the > > vertical scrollbar does move, the horizontal one (which Bert just added) > > doesn't. So I think there is a slight difference in how the horizontal > > scrollbar is set up that is causing this. > > On second thought, wouldn't it make more sense to expand the commit > message buffer instead? The point of resizing that pane is to see more > of the commit message. So it makes more sense to make the commit message > buffer take up all the vertical space, rather than making the scrollbar > move. it is, I just broke that ;-) Bert > > -- > Regards, > Pratyush Yadav
Re: feature request, git-gui: add hotkey to toggle amend/new
Birger, On Mon, Sep 2, 2019 at 6:25 PM Birger Skogeng Pedersen wrote: > > I just now realized what a terrible suggestion CTRL+Z was. > I propose CTRL/CMD+E to toggle between amend/new commit. > > On Sat, Aug 31, 2019 at 12:51 PM Birger Skogeng Pedersen > wrote: > > > > In my pursuit to fully utilize git-gui with only using a keyboard, I > > suggest that there is a hotkey to toggle between selecting "New > > Commit" and "Amend Last Commit". > > > > Not sure which key-combination that fits this purpose best, but my > > suggestion is CTRL/CMD+Z. I think with your "focus" patch, this is not needed anymore: After focusing the commit message widget, you can focus the radio buttons with Tab/Shift+Tab and press Space. I think this is short enough, so that wasting a Letter is not justified here. Best, Bert > > > > Best regards, > > Birger
Re: git-gui: Long lines in commit message gets hidden, no scrollbar appears
On Mon, Sep 2, 2019 at 7:35 PM Junio C Hamano wrote: > > Bert Wesarg writes: > > > the old reasoning was, that you should not create commit messages > > which are too wide. > > 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. Bert
Re: git-gui: Long lines in commit message gets hidden, no scrollbar appears
Hi Birger,
On Mon, Sep 2, 2019 at 11:13 AM Birger Skogeng Pedersen
wrote:
>
> Hi,
>
> I just noticed that long lines in the commit message widget does in
> fact not show a horizontal scrollbar.
> So if a line in the commit message is more than 75 characters, it gets
> a bit confusing.
> Should it not have a scrollbar?
> Example shown here: https://i.imgur.com/I3d6nBJ.png
the old reasoning was, that you should not create commit messages
which are too wide. While I can follow this reasoning, hiding content
is also not a good idea. Thus I tried a different aproach. Instead of
not showing the content, I pop up a dialog, if the commit message
contains too long lines and add you requested scrollbar.
As I'm currently unable to send patches per mail, I pushed it to GitHub:
https://github.com/bertwesarg/git-gui/tree/bw/commit-scrollbuffer
And I will paste the scrollbuffer commit below.
Best,
Bert
Author: Bert Wesarg
Date: Mon Sep 2 13:57:24 2019 +0200
git-gui: add horizontal scrollbar to commit buffer
Sugestted-by: Birger Skogeng Pedersen
Signed-off-by: Bert Wesarg
diff --git a/git-gui.sh b/git-gui.sh
index a491085..919d1e9 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 \
- -yscrollcommand {.vpane.lower.commarea.buffer.frame.sby set}
+ -yscrollcommand {.vpane.lower.commarea.buffer.frame.sby set} \
+ -xscrollcommand {.vpane.lower.commarea.buffer.sbx set}
${NS}::scrollbar .vpane.lower.commarea.buffer.frame.sby \
+ -orient vertical \
-command [list $ui_comm yview]
+${NS}::scrollbar .vpane.lower.commarea.buffer.sbx \
+ -orient horizontal \
+ -command [list $ui_comm xview]
pack .vpane.lower.commarea.buffer.frame.sby -side right -fill y
pack $ui_comm -side left -fill y
+pack .vpane.lower.commarea.buffer.sbx -side bottom -fill x
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 top -fill x
pack .vpane.lower.commarea.buffer -side left -fill y
# -- Commit Message Buffer Context Menu
Re: [PATCH] git-gui: Add hotkeys to set widget focus
On Sun, Sep 1, 2019 at 1:32 PM Pratyush Yadav wrote:
> > +
> > +proc select_first_unstaged_changes_path {} {
> > + global ui_workdir
> > + select_first_path $ui_workdir
> > +}
> > +
> > +proc select_first_staged_changes_path {} {
> > + global ui_index
> > + select_first_path $ui_index
> > +}
> > +
> > +proc focus_diff {} {
> > + global ui_diff
> > + focus $ui_diff
> > +}
> > +
> > +proc focus_commit_message {} {
> > + global ui_comm
> > + focus $ui_comm
> > +}
> > +
>
> Do you expect these functions to be re-used somewhere in the near
> future? Otherwise...
>
> > ##
> > ##
> > ## ui construction
> > @@ -3877,6 +3924,14 @@ foreach i [list $ui_index $ui_workdir] {
> > }
> > unset i
> >
> > +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}
> > +bind . <$M1B-Key-2> {select_first_staged_changes_path}
> > +bind . <$M1B-Key-3> {focus_diff}
> > +bind . <$M1B-Key-4> {focus_commit_message}
>
> ... why not just put their bodies directly in here? Something like:
>
> bind . <$M1B-Key-1> {
> global $ui_workdir
> select_first_path $ui_workdir
> }
that should also work:
bind . <$M1B-Key-1> {select_first_path $::ui_workdir}
Re: [PATCH] git-gui: Add hotkeys to set widget focus
Hi Birger,
On Sat, Aug 31, 2019 at 2:23 PM Birger Skogeng Pedersen
wrote:
>
> The user cannot change focus between the list of files, the diff view and
> the commit message widgets without using the mouse (clicking either of
> the four widgets ).
that bugged me two, but never come up with a good idea.
>
> Hotkeys CTRL/CMD+number (1-4) now focuses a previously selected path from
> either the "Unstaged Changes" or "Staged Changes", the diff view or the
> commit message dialog widgets, respectively. This enables the user to
> select/unselect files, view the diff and create a commit in git-gui
> using keyboard-only.
But I don't understand this in full. Does this mean pressing CTRL+1 or
+2 does also changes the file selection? Why isn't it sufficient to
just focus the respective file list widget? And than have bindings to
change the selection?
Bert
>
> Signed-off-by: Birger Skogeng Pedersen
> ---
> git-gui/git-gui.sh | 57 +-
> 1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 6de74ce639..cbd0b69804 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -2494,7 +2494,7 @@ proc force_first_diff {after} {
>
> proc toggle_or_diff {mode w args} {
> global file_states file_lists current_diff_path ui_index ui_workdir
> - global last_clicked selected_paths
> + global last_clicked selected_paths file_lists_last_clicked
>
> if {$mode eq "click"} {
> foreach {x y} $args break
> @@ -2551,6 +2551,8 @@ proc toggle_or_diff {mode w args} {
> $ui_index tag remove in_sel 0.0 end
> $ui_workdir tag remove in_sel 0.0 end
>
> + set file_lists_last_clicked($w) $lno
> +
> # Determine the state of the file
> if {[info exists file_states($path)]} {
> set state [lindex $file_states($path) 0]
> @@ -2664,6 +2666,51 @@ proc show_less_context {} {
> }
> }
>
> +proc select_first_path {w} {
> + global file_lists last_clicked selected_paths ui_workdir
> + global file_lists_last_clicked
> +
> + set _list_length [llength $file_lists($w)]
> +
> + if {$_list_length > 0} {
> +
> + set _index $file_lists_last_clicked($w)
> +
> + if {$_index eq {}} {
> + set _index 1
> + } elseif {$_index > $_list_length} {
> + set _index $_list_length
> + }
> +
> + focus $w
> + set last_clicked [list $w $_index]
> + set path [lindex $file_lists($w) [expr $_index - 1]]
> + array unset selected_paths
> + set selected_paths($path) 1
> + show_diff $path $w
> + }
> +}
> +
> +proc select_first_unstaged_changes_path {} {
> + global ui_workdir
> + select_first_path $ui_workdir
> +}
> +
> +proc select_first_staged_changes_path {} {
> + global ui_index
> + select_first_path $ui_index
> +}
> +
> +proc focus_diff {} {
> + global ui_diff
> + focus $ui_diff
> +}
> +
> +proc focus_commit_message {} {
> + global ui_comm
> + focus $ui_comm
> +}
> +
> ##
> ##
> ## ui construction
> @@ -3877,6 +3924,14 @@ foreach i [list $ui_index $ui_workdir] {
> }
> unset i
>
> +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}
> +bind . <$M1B-Key-2> {select_first_staged_changes_path}
> +bind . <$M1B-Key-3> {focus_diff}
> +bind . <$M1B-Key-4> {focus_commit_message}
> +
> +set file_lists_last_clicked($ui_index) {}
> +set file_lists_last_clicked($ui_workdir) {}
> +
> set file_lists($ui_index) [list]
> set file_lists($ui_workdir) [list]
>
> --
> 2.23.0.37.g745f681289
>
Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines
On Sat, Aug 24, 2019 at 1:43 AM David Aguilar wrote: > > I have a very strong opinion about the confirmation dialog, so I'll just > mention that here since Hannes is on this thread. > > In cola we do have a confirmation dialog, and I strongly believe this is > the correct behavior because it's an operation that drops data that > cannot be recovered. > > In the other thread, it was mentioned that this dialog would be a > nuisance. Perhaps that is true -- for the dialog that may have been > implemented in this series (I haven't run it to verify). > > Let's dive into that concern. > > In git-cola we have a confirmation dialog and it is by no way a > detriment to the workflow, and I use that feature all the time. > Why? The reason is that we focused on the keyboard interaction. > > The workflow is as follows: > > Ctrl-u to initiate the revert action > The prompt appears immediately. > - Hitting any of "enter", "y", or "spacebar" will > confirm the confirmation, and proceed. > - Hitting any of "escape" or "n" will cancel the action. > > So essentially the workflow for the power user becomes "ctrl-u, enter" > and that is such a tiny overhead that it really is not a bother at all. > > On the other hand, if I had to actually move my hand over to a mouse or > trackpad and actually "click" on something then I would be super > annoyed. That would be simply horrible with RSI in mind. > I take this as a point for*not* having a confirmation dialog when doing the action per mouse. Which matches exactly my original implementation. > OTOH having to hit "enter" or "spacebar" (which is the largest key on > your keyboard, and your thumbs have good hefty muscles) is totally > acceptable in my book because it strikes the right balance between > safety for a destructive operation and convenience. > > Now, let's consider the alternative -- adding an option to disable the > prompt. I don't like that. > > Why? It's yet another option. It's yet another thing to document, yet > another code path, and yet another pitfall for a user who might run > git-gui in a different configuration (and becomes surprised when revert > doesn't prompt and suddenly loses their work). > > Do we really need an option, or do we need better usability instead? > My opinion is that the latter is the real need. > > > That's my $.02 from having used this feature in practice since 2013. 2012 Best, Bert > -- > David
Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines
On Sat, Aug 24, 2019 at 1:43 AM David Aguilar wrote: > > On Fri, Aug 23, 2019 at 03:31:03AM +0530, Pratyush Yadav wrote: > > Hi, > > > > This series adds the ability to revert selected lines and hunks in > > git-gui. Partially based on the patch by Bert Wesarg [0]. > > > > The commits can be found in the topic branch 'py/revert-hunks-lines' > > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > > > Once reviewed, pull the commits from > > 415ce3f8582769d1d454b3796dc6c9c847cefa87 till > > 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and > > apply them locally, whichever you prefer. > > > > Changes in v2: > > - Add an option to disable the revert confirmation prompt as suggested > > by Johannes Sixt. > > - Base the patches on Pat's git-gui tree instead of git.git. > > > We've had these features for years in git-cola. this series does not introduce new hotkeys. > > Please copy our keyboard shortcuts. > IMO we should not re-invent the user interactions. > > http://git-cola.github.io/share/doc/git-cola/hotkeys.html my part for the homework: > > Ctrl-u is our revert-unstaged-edits hotkeys. https://github.com/patthoyts/git-gui/commit/b677c66e299c8754a6093cbd19ce71b0ad2a8a17 > "s" is for > staging/unstaging (or Ctrl-s if the file list is focused). https://github.com/patthoyts/git-gui/commit/cd16a6c9298778265a044e5f9a39b006277b32f2 https://github.com/patthoyts/git-gui/commit/e210e67451f22f97c1476d6b78b6fa7fdd5817f9#diff-ceba4b88c7e634c5401a4487d45d3ab4R774 Bert > > The same hotkey is used for operating at the line level. > If no lines are selected, the hunk surrounding the current cursor > position is used. > > Please make keyboard interaction a first-class design consideration. >
Re: [PATCH v2 3/4] git-gui: Add the ability to revert selected lines
On Fri, Aug 23, 2019 at 12:01 AM Pratyush Yadav wrote:
>
> Just like the user can select lines to stage or unstage, add the
> ability to revert selected lines.
>
> Signed-off-by: Pratyush Yadav
> ---
> git-gui.sh | 25 -
> lib/diff.tcl | 31 ++-
> 2 files changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index b7811d8..9d84ba9 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3588,9 +3588,15 @@ set ui_diff_applyhunk [$ctxm index last]
> lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state]
> $ctxm add command \
> -label [mc "Apply/Reverse Line"] \
> - -command {apply_range_or_line $cursorX $cursorY; do_rescan}
> + -command {apply_or_revert_range_or_line $cursorX $cursorY 0;
> do_rescan}
> set ui_diff_applyline [$ctxm index last]
> lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state]
> +$ctxm add command \
> + -label [mc "Revert Line"] \
> + -command {apply_or_revert_range_or_line $cursorX $cursorY 1;
> do_rescan}
> +set ui_diff_revertline [$ctxm index last]
> +lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state]
> +set ui_diff_revertline [$ctxm index last]
> $ctxm add separator
> $ctxm add command \
> -label [mc "Show Less Context"] \
> @@ -3688,15 +3694,19 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
> set l [mc "Unstage Hunk From Commit"]
> if {$has_range} {
> set t [mc "Unstage Lines From Commit"]
> + set r [mc "Revert Lines"]
> } else {
> set t [mc "Unstage Line From Commit"]
> + set r [mc "Revert Line"]
> }
> } else {
> set l [mc "Stage Hunk For Commit"]
> if {$has_range} {
> set t [mc "Stage Lines For Commit"]
> + set r [mc "Revert Lines"]
> } else {
> set t [mc "Stage Line For Commit"]
> + set r [mc "Revert Line"]
> }
> }
> if {$::is_3way_diff
> @@ -3707,11 +3717,24 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
> || [string match {T?} $state]
> || [has_textconv $current_diff_path]} {
> set s disabled
> + set revert_state disabled
> } else {
> set s normal
> +
> + # Only allow reverting changes in the working tree. If
> + # the user wants to revert changes in the index, they
> + # need to unstage those first.
> + if {$::ui_workdir eq $::current_diff_side} {
> + set revert_state normal
> + } else {
> + set revert_state disabled
> + }
> }
> +
> $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l
> $ctxm entryconf $::ui_diff_applyline -state $s -label $t
> + $ctxm entryconf $::ui_diff_revertline -state $revert_state \
> + -label $r
so you have the 'Revert Line(s)' menu entry always in the context
menu, also when the context menu was opened for an staged file (than
the menu entry is only disabled)? I think this is a step backwards
from my original patch (which isn't mentioned/referenced at all in
this patch anyway).
My orignal patch also had this hunk in lib/diff.tcl:
@@ -614,6 +619,8 @@ proc apply_hunk {x y} {
if {$current_diff_side eq $ui_index} {
set mi ${o}M
+} elseif {$revert} {
+set mi "[string index $mi 0]$o"
} elseif {[string index $mi 0] eq {_}} {
set mi M$o
} else {
which is missing here. I cannot remember why I added this here. But
maybe you can?
Best,
Bert
> tk_popup $ctxm $X $Y
> }
> }
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 4cae10a..00b15f5 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -640,7 +640,7 @@ proc apply_hunk {x y} {
> }
> }
>
> -proc apply_range_or_line {x y} {
> +proc apply_or_revert_range_or_line {x y revert} {
> global current_diff_path current_diff_header current_diff_side
> global ui_diff ui_index file_states
>
> @@ -660,25 +660,46 @@ proc apply_range_or_line {x y} {
> if {$current_diff_path eq {} || $current_diff_header eq {}} return
> if {![lock_index apply_hunk]} return
>
> - set apply_cmd {apply --cached --whitespace=nowarn}
> + set apply_cmd {apply --whitespace=nowarn}
> set mi [lindex $file_states($current_diff_path) 0]
>
Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines
On Fri, Aug 23, 2019 at 12:51 AM Pratyush Yadav wrote: > > On 22/08/19 03:34PM, Junio C Hamano wrote: > > Pratyush Yadav writes: > > > > > This series adds the ability to revert selected lines and hunks in > > > git-gui. Partially based on the patch by Bert Wesarg [0]. > > > > > > The commits can be found in the topic branch 'py/revert-hunks-lines' > > > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > > > > > Once reviewed, pull the commits from > > > 415ce3f8582769d1d454b3796dc6c9c847cefa87 till > > > 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and > > > apply them locally, whichever you prefer. > > > > Let's see how we can work together by you playing the role of > > git-gui maintainer and the others on the list (including me) playing > > the role of reviewer and contributor. So I may keep an eye on the > > discussion on this thread, I may even comment on them myself, but > > you'll be the one waiting for the discussion to settle, adjusting > > the patches in response to reviews, etc. and making the final > > decision when/if the topic is done, at which time you'd be telling > > me to pull from you. > > > > > Pratyush Yadav (4): > > > git-gui: Move revert confirmation dialog creation to separate function > > > git-gui: Add option to disable the revert confirmation prompt > > > git-gui: Add the ability to revert selected lines > > > git-gui: Add the ability to revert selected hunk > > > > "Move" and "Add" after "git-gui:" would better be downcased to be > > more in line with the others in "git shortlog --no-merges"; I also > > think "allow doing X" is shorter and better way to say "add the > > ability to do X". > > Will fix. Thanks. > > > If I am reading the first patch correctly, we already ask for > > confirmation before reverting local changes, and the steps 3 and 4 > > are about allowing partial reversion in addition to the wholesale > > reversion, right? > > Yes. The ability to revert whole files already exists. This revert > always asks for confirmation. Steps 3 and 4 allow for partial reverts. > Step 2 allows the user to disable the confirmation dialog for both > whole-file reverts and for partial reverts. > > > An earlier objection from j6t sounded like we > > require users to respond to an extra dialog after this series, but > > that does not look like the case. Instead, step 2 adds a new > > feature to allow those to opt-out of the existing dialog (which may > > be reused to squelch the dialog to protect features added in steps 3 > > and 4). Am I reading the series correctly? > > Yes. The users always responded to an extra dialog for whole file > reverts even before these changes. j6t was running a fork of git-gui > which had the ability for partial reverts, and his fork did not ask for > confirmation for partial reverts. Always asking for confirmation > disrupts his workflow, so I added an option to disable it. It disables > the dialog for partial reverts (which j6t cares about), and also for > whole file reverts, which I added to maintain consistency. as I'm the one who use this feature for more than 7 years, I can only object to this. I'm happy to have the confirmation dialog for the whole-file revert (the current state) but having the dialog also for partial revert would be too disruptive. And disabling both at the same time would not be an option for me. The thing is, that the partial revert "just don't happen by accident". Here are the minimum user actions needed to get to this dialog: 1. whole-file revert - do a Ctrl+J, more or less anywhere in the GUI 2. hunk revert/revert one unselected line - right click anywhere in the diff pane (thats around 60% of the window area) - move the mouse pointer down 3/4 menu items - click this menu item 3. partially revert selected lines - select some content in the diff pane by starting by pressing and holding a left click - end the selection by releasing the left click - move the mouse pointer down 3/4 menu items - click this menu item Thats always at least 2 user actions more than the whole-file revert. Thus this cannot happen by accident quite easily in comparison to the whole-file revert. And thats the reason why this dialog exists, from my point of view. I can see the need to disable the dialog for the whole-file revert, and IIRC that was also requested a long time ago on this list. But I don't see a reason to have this dialog also for the partial reverts as a safety measure. Best, Bert > > -- > Regards, > Pratyush Yadav
Re: [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines
Dear Pratyush, putting me in Cc would have been nice :( I will look into your patches in the comming hours. Bert On Mon, Aug 19, 2019 at 11:41 PM Pratyush Yadav wrote: > > Hi, > > This series adds the ability to revert selected lines and hunks in > git-gui. Partially based on the patch by Bert Wesarg [0]. > > The commits can be found in the topic branch 'py/git-gui-revert-lines' > at https://github.com/prati0100/git/tree/py/git-gui-revert-lines > > Once reviewed, pull the commits from > 832064f93d3ad432616e96ca70f682a7cfdcc3e0 till > 72eed27a29f1e71cbefefa6b19f96c89793ac74d. > > Let me know if there is some other way I'm supposed to ask for a pull. > > [0] > https://public-inbox.org/git/a9ba4550a29d7f3c653561e7029f0920bf8eb008.1326116492.git.bert.wes...@googlemail.com/ > > Pratyush Yadav (3): > git-gui: Move revert confirmation dialog creation to separate function > git-gui: Add the ability to revert selected lines > git-gui: Add the ability to revert selected hunk > > git-gui/git-gui.sh| 39 -- > git-gui/lib/diff.tcl | 65 --- > git-gui/lib/index.tcl | 27 +++--- > 3 files changed, 109 insertions(+), 22 deletions(-) > > -- > 2.21.0 >
Re: git-gui: unstaged changes windowpane resets after unstaging a line
On Sun, Mar 31, 2019 at 9:48 PM Jan Ziak <0xe2.0x9a.0...@gmail.com> wrote: > > On Sun, 31 Mar 2019 at 21:15, Bert Wesarg wrote: > > > > Hi Jan, > > > > On Sun, Mar 31, 2019 at 8:45 PM Jan Ziak <0xe2.0x9a.0...@gmail.com> wrote: > > > > > > Hello > > > > > > After performing "Unstage Line From Commit" action, the "Unstaged > > > Changes" windowpane is reset. The reset does not happen with "Unstage > > > Hunk From Commit" action. > > > > because its not necessary when staging a hunk. Though when staging a > > line it is better to run the diff algorithm again. > > Yes. I was thinking comparing the old state of "Unstaged Changes" to > the new state and preserving (or approximating) the selected filename > and scroller position if possible. > > When unstaging a line (or a hunk) there is internally no possibility > for the currently selected filename to disappear from the "Unstaged > Changes" list. > > > Anyway, which problem do you observe in particular? > > The problem is that the selected filename and scroller position are > reinitialized. It is uncomfortable when "Unstaged Changes" contains a > lot of filenames. I can't confirm this behavior. Neither when staging lines from the "Unstaged Changes" file list, nor when unstaging lines from the "Stages Changes (Will Commit)" file list. Could you try to come up with a recipe from an empty repository? Best, Bert > > Sincerely > Jan > >
Re: git-gui: unstaged changes windowpane resets after unstaging a line
Hi Jan, On Sun, Mar 31, 2019 at 8:45 PM Jan Ziak <0xe2.0x9a.0...@gmail.com> wrote: > > Hello > > After performing "Unstage Line From Commit" action, the "Unstaged > Changes" windowpane is reset. The reset does not happen with "Unstage > Hunk From Commit" action. because its not necessary when staging a hunk. Though when staging a line it is better to run the diff algorithm again. Anyway, which problem do you observe in particular? Best, Bert > > git version 2.21.0 > > Sincerely > Jan
Re: [RFC PATCH] Introduce "precious" file concept
On Sun, Nov 11, 2018 at 10:53 AM Nguyễn Thái Ngọc Duy wrote: > > Also while "precious" is a fun name, but it does not sound serious. > Any suggestions? Perhaps "valuable"? "precious" is also used by POSIX make: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/make.html Bert
Re: git-gui ignores core.hooksPath
On Wed, Apr 11, 2018 at 12:50 AM, Junio C Hamano wrote: > Chris Maes writes: > >> Is there any hope from here that anyone will pick up this / these >> changes? Will anyone else be assigned the main responsible for this >> git-gui repository? >> >> Just hoping to revive the discussion here, since the >> https://github.com/patthoyts/git-gui/ repository seems quite dead. > > It indeed does. > > I've played a patch-monkey in the past for git-gui and have a few > topics queued still in my tree, but that serves merely as a bookmark > that is slightly better than a pointer to the mailing list archive. > > We need a volunteer to take over this part of the subsystem; > somebody who actually uses it, passionate about improving it, and > can speak tcl/tk somewhat fluently (I qualify none of these three > criteria myself). > > Any takers? the last time this topic came up, Stefan (in Cc) offered to volunteer. Stefan, is this offer still open? I would support this. Best,
Re: [PATCH] gitk: do not limit tree mode listing in the file list panel to current sub-directory
Thanks.
On Wed, May 9, 2018 at 12:59 PM, Alex Riesen
wrote:
> From: Alex Riesen
>
> The previous behavior conflicts with the "Patch" mode of the panel,
> which always shows the changes from the top-level of the repository.
> It is also impossible to get back to the full listing without restarting
> gitk.
> ---
>
> Bert Wesarg, Wed, May 09, 2018 09:19:55 +0200:
>> > Frankly, this listing limited to just a sub-directory confuses me a bit. Is
>> > there anyway to get to display full repository without changing to the top
>> > level?
>>
>> I noticed that too, while testing your patch and I'm also confused.
>> But was not able to send a request to Paul yet. ls-tree --full-tree
>> seems to be one that should be used here, I think.
>
> Well, I just tried your suggestion. 'ls-files' doesn't have --full-tree, so
> for those it is just cd-up.
>
> It is on top of the re-sent series.
I would consider the current behavior as a bug, therefor I would put
this patch first, and than your other fixes/enhancements.
>
> gitk | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gitk b/gitk
> index c430dfe..03ead98 100755
> --- a/gitk
> +++ b/gitk
> @@ -7600,18 +7600,18 @@ proc go_to_parent {i} {
>
> proc gettree {id} {
> global treefilelist treeidlist diffids diffmergeid treepending
> -global nullid nullid2
> +global nullid nullid2 cdup
>
> set diffids $id
> unset -nocomplain diffmergeid
> if {![info exists treefilelist($id)]} {
> if {![info exists treepending]} {
> if {$id eq $nullid} {
> - set cmd [list | git ls-files]
> + set cmd [list | git -C $cdup ls-files]
> } elseif {$id eq $nullid2} {
> - set cmd [list | git ls-files --stage -t]
> + set cmd [list | git -C $cdup ls-files --stage -t]
> } else {
> - set cmd [list | git ls-tree -r $id]
> + set cmd [list | git ls-tree --full-tree -r $id]
> }
> if {[catch {set gtf [open $cmd r]}]} {
> return
> @@ -7670,7 +7670,7 @@ proc gettreeline {gtf id} {
> proc showfile {f} {
> global treefilelist treeidlist diffids nullid nullid2
> global ctext_file_names ctext_file_lines
> -global ctext commentend
> +global ctext commentend cdup
>
> set submodlog "log --format=%h\\ %aN:\\ %s -100"
> set fcmt ""
> @@ -7680,15 +7680,15 @@ proc showfile {f} {
> return
> }
> if {$diffids eq $nullid} {
> - if {[file isdirectory $f]} {
> + if {[file isdirectory "$cdup$f"]} {
> # a submodule
> - set qf [shellquote $f]
> + set qf [shellquote "$cdup$f"]
> if {[catch {set bf [open "| git -C $qf $submodlog" r]} err]} {
> puts "oops, can't read submodule $f: $err"
> return
> }
> } else {
> - if {[catch {set bf [open $f r]} err]} {
> + if {[catch {set bf [open "$cdup$f" r]} err]} {
> puts "oops, can't read $f: $err"
> return
> }
> @@ -7704,7 +7704,7 @@ proc showfile {f} {
> }
> } else {
> # also a submodule
> - set qf [shellquote $f]
> + set qf [shellquote "$cdup$f"]
> if {[catch {set bf [open "| git -C $qf $submodlog $blob" r]}
> err]} {
> puts "oops, error reading submodule commit: $err"
> return
> --
> 2.17.0.593.g2029711e64
>
>
> ---
> Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
> https://www.avast.com/antivirus
>
Re: [PATCH 2/2] gitk: add an option to run gitk on an item in the file list
On Tue, May 8, 2018 at 3:39 PM, Alex Riesen
wrote:
> Bert Wesarg, Tue, May 08, 2018 15:17:03 +0200:
>> On Tue, May 8, 2018 at 2:22 PM, Alex Riesen
>> wrote:
>> > +proc flist_gitk {} {
>> > +global flist_menu_file findstring gdttype
>> > +
>> > +set x [shellquote $flist_menu_file]
>>
>> this needs to handle cdup, i.e., if gitk is run from a subdirectory,
>> all paths needs to be prefixed with $cdup, like: [file join $cdup
>> $flist_menu_file]
>
> That, indeed, is easily done:
>
> set x [shellquote [file join $cdup $flist_menu_file]]
> if {[file isdirectory $flist_menu_file]} {
> exec sh -c "cd $x&&gitk" &
> } else {
> exec gitk -- $x &
> }
>
>
> It just doesn't seem to work: gitk does not find any commits!
> Maybe that's because the file panel lists only files for the current
> subdirectory (without the path from the repo top-level)? E.g.
>
> mkdir tst
> cd tst
> git init
> mkdir a
> touch top-file a/sub-file
> git add -A ; git commit -m.
> cd a
> gitk
>
> Gitk lists only sub-file.
>
> Frankly, this listing limited to just a sub-directory confuses me a bit. Is
> there anyway to get to display full repository without changing to the top
> level?
I noticed that too, while testing your patch and I'm also confused.
But was not able to send a request to Paul yet. ls-tree --full-tree
seems to be one that should be used here, I think.
Bert
Re: [PATCH 2/2] gitk: add an option to run gitk on an item in the file list
On Tue, May 8, 2018 at 2:22 PM, Alex Riesen
wrote:
> From: Alex Riesen
>
> Similar to a git gui feature which visualizes history in a submodule,
> the submodules cause the gitk be started inside the submodule.
>
> Signed-off-by: Alex Riesen
> ---
> gitk | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/gitk b/gitk
> index d34833f..1ec545e 100755
> --- a/gitk
> +++ b/gitk
> @@ -2682,6 +2682,7 @@ proc makewindow {} {
> {mc "External diff" command {external_diff}}
> {mc "Blame parent commit" command {external_blame 1}}
> {mc "Copy path" command {clipboard clear; clipboard append
> $flist_menu_file}}
> + {mc "Run gitk on this" command {flist_gitk}}
> }
> $flist_menu configure -tearoff 0
>
> @@ -3555,6 +3556,17 @@ proc flist_hl {only} {
> set gdttype [mc "touching paths:"]
> }
>
> +proc flist_gitk {} {
> +global flist_menu_file findstring gdttype
> +
> +set x [shellquote $flist_menu_file]
this needs to handle cdup, i.e., if gitk is run from a subdirectory,
all paths needs to be prefixed with $cdup, like: [file join $cdup
$flist_menu_file]
Bert
> +if {[file isdirectory $flist_menu_file]} {
> + exec sh -c "cd $x&&gitk" &
> +} else {
> + exec gitk -- $x &
> +}
> +}
> +
> proc gitknewtmpdir {} {
> global diffnum gitktmpdir gitdir env
>
includeIf breaks calling dashed externals
Dear Duy, heaving an includeIf in a git config file breaks calling external git commands, most prominently git-gui. $ git --version git version 2.12.2.599.gcf11a6797 $ git rev-parse --is-inside-work-tree true $ git echo git: 'echo' is not a git command. See 'git --help'. Did you mean this? fetch $ echo '[includeIf "gitdir:does-not-exists"]path = does-not-exists' >>.git/config $ git rev-parse --is-inside-work-tree true $ git echo fatal: BUG: setup_git_env called without repository Best, Bert
Re: [RFC][Git GUI] Make Commit message field in git GUI re sizable.
HI, the reason why it is fixed, is because commit messages should be wrapped at 76 characters to be used in mails. So it helps you with the wrapping. Bert On Wed, Feb 22, 2017 at 10:27 AM, Jessie Hernandez wrote: > Hi all, > > I have been using git for a few years now and really like the software. > I have a small annoyance and was wondering if I could get the communities > view on this. > > When using git GUI I find it handy to be able to re-size the "Unstaged > Changes" and the "Staged Changed" fields. > > I would like the same thing for the "Commit Message" field, or to have it > re-size with the git GUI window. > > I can re-size the "Commit Message" vertically when making the "Modified" > panel smaller. > > Does this make sense? > I would be happy to get into more detail if that is necessary or if > something is not clear. > > Thank you. > > - > Jessie Hernandez > >
Re: [wish] Revert changes in git gui
On Fri, Dec 12, 2014 at 9:27 AM, Christoph Grüninger wrote: > Hi Bert, > your commit is more than half a year old. Do you intent to include that > into Git master? If not, what's the reason? Thats a really odd question to a person who posted this patch to the mailling list the fist place, isn't it? If anything you should have asked the git gui developers and community, why they didn't show interest to have this in master, right? Bert > > Bye > Christoph > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [wish] Revert changes in git gui
On Tue, Dec 9, 2014 at 9:21 PM, Johannes Sixt wrote: > Am 09.12.2014 um 20:49 schrieb Christoph Grüninger: >> While browsing the changes, it is very easy to add (or remove) lines or >> hunks for commit via the context menu. I would like to revert the >> changes of a line or a hunk in a similar way. I have often white-space >> or formatting changes I don't want to commit but want them reverted >> immediately. > > I'm using this patch series since it was posted: > > http://thread.gmane.org/gmane.comp.version-control.git/188170/focus=188171 Maybe this is easier to apply: http://repo.or.cz/w/git-gui/bertw.git/commit/5d7a81b626c34a94c6c43937d8a64572b6231196 Bert -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/18] the beginning of the signed push
On Wed, Aug 20, 2014 at 12:06 AM, Junio C Hamano wrote: > The basic flow based on this mechanism goes like this: > > 1. You push out your work with "git push -s". > > 2. The sending side learns where the remote refs are as usual, > together with what protocol extension the receiving end > supports. If the receiving end does not advertise the protocol > extension "push-cert", the sending side falls back to the normal > push that is not signed. > Is this fallback silently? If so I think it would be better to abort the push, if the receiver does not support this but the user requested it. Bert -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Add a little script to compare two make perf runs
On Sun, Jul 6, 2014 at 6:15 PM, Andi Kleen wrote: >> a justification why the geometric mean is used here would increase my >> confident significantly. > > It's just a standard way to summarize sets of benchmarks. For example SPEC > uses the same approach. No, SPEC would have calculated the geometric mean of the ratios cmp[1][j] / cmp[0][j]. And this should also only be used under the assumption that there is a multiplicative correlation. Bert -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Add a little script to compare two make perf runs
Hi,
On Sat, Jul 5, 2014 at 1:43 AM, Andi Kleen wrote:
> From: Andi Kleen
>
> Signed-off-by: Andi Kleen
> ---
> diff-res | 26 ++
> 1 file changed, 26 insertions(+)
> create mode 100755 diff-res
>
> diff --git a/diff-res b/diff-res
> new file mode 100755
> index 000..90d57be
> --- /dev/null
> +++ b/diff-res
> @@ -0,0 +1,26 @@
> +#!/usr/bin/python
> +# compare two make perf output file
> +# this should be the results only without any header
> +import argparse
> +import math, operator
> +from collections import OrderedDict
> +
> +ap = argparse.ArgumentParser()
> +ap.add_argument('file1', type=argparse.FileType('r'))
> +ap.add_argument('file2', type=argparse.FileType('r'))
> +args = ap.parse_args()
> +
> +cmp = (OrderedDict(), OrderedDict())
> +for f, k in zip((args.file1, args.file2), cmp):
> +for j in f:
> +num = j[59:63]
> +name = j[:59]
> +k[name] = float(num)
> +
> +for j in cmp[0].keys():
> +print j, cmp[1][j] - cmp[0][j]
> +
> +def geomean(l):
> + return math.pow(reduce(operator.mul, filter(lambda x: x != 0.0, l)), 1.0
> / len(l))
> +
> +print "geomean %.2f -> %.2f" % (geomean(cmp[0].values()),
> geomean(cmp[1].values()))
a justification why the geometric mean is used here would increase my
confident significantly.
It calculates wrong values anyway iff there are zeros in the sampling set.
Thanks.
Bert
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-http: use argv-array
On Tue, Jul 9, 2013 at 7:18 AM, Junio C Hamano wrote:
> Instead of using a hand-managed argument array, use argv-array API
> to manage dynamically formulated command line.
>
> Signed-off-by: Junio C Hamano
> ---
> remote-curl.c | 31 +++
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index 60eda63..884b3a3 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -7,6 +7,7 @@
> #include "run-command.h"
> #include "pkt-line.h"
> #include "sideband.h"
> +#include "argv-array.h"
>
> static struct remote *remote;
> static const char *url; /* always ends with a trailing slash */
> @@ -787,36 +788,34 @@ static int push_dav(int nr_spec, char **specs)
> static int push_git(struct discovery *heads, int nr_spec, char **specs)
> {
> struct rpc_state rpc;
> - const char **argv;
> - int argc = 0, i, err;
> + int i, err;
> + struct argv_array args;
> +
> + argv_array_init(&args);
> + argv_array_pushl(&args, "send-pack", "--stateless-rpc",
> "--helper-status");
missing NULL sentinel. GCC has the 'sentinel' [1] attribute to catch
such errors. Or use macro magic:
void argv_array_pushl_(struct argv_array *array, ...);
#define argv_array_pushl(array, ...) argv_array_pushl_(array, __VA_ARGS__, NULL)
Bert
[1]
http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html#index-g_t_0040code_007bsentinel_007d-function-attribute-2708
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] shorten_unambiguous_ref(): Allow shortening refs/remotes/origin/HEAD to origin
On Sun, May 5, 2013 at 1:55 AM, Johan Herland wrote:
> When expanding shorthand refs to full ref names (e.g. in dwim_ref()),
> we use the ref_rev_parse_rules list of expansion patterns. This list
> allows "origin" to be expanded into "refs/remotes/origin/HEAD", by
> using the "refs/remotes/%.*s/HEAD" pattern from that list.
>
> shorten_unambiguous_ref() exists to provide the reverse operation:
> turning a full ref name into a shorter (but still unambiguous) name.
> It does so by matching the given refname against each pattern from
> the ref_rev_parse_rules list (in reverse), and extracting the short-
> hand name from the matching rule.
>
> However, when given "refs/remotes/origin/HEAD" it fails to shorten it
> into "origin", because we misuse the sscanf() function when matching
> "refs/remotes/origin/HEAD" against "refs/remotes/%.*s/HEAD": We end
> up calling sscanf like this:
>
> sscanf("refs/remotes/origin/HEAD", "refs/remotes/%s/HEAD", short_name)
>
> In this case, sscanf() will match the initial "refs/remotes/" part, and
> then match the remainder of the refname against the "%s", and place it
> ("origin/HEAD") into short_name. The part of the pattern following the
> "%s" format is never verified, because sscanf() apparently does not
> need to do that (it has performed the one expected format extraction,
> and will return 1 correspondingly; see [1] for more details).
>
> This patch replaces the misuse of sscanf() with a fairly simple function
> that manually matches the refname against patterns, and extracts the
> shorthand name.
>
> Also a testcase verifying "refs/remotes/origin/HEAD" -> "origin" has
> been added.
>
> [1]: If we assume that sscanf() does not do a verification pass prior
> to format extraction, there is AFAICS _no_ way for sscanf() - having
> already done one or more format extractions - to indicate to its caller
> that the input fails to match the trailing part of the format string.
> In other words, AFAICS, the scanf() family of function will only verify
> matching input up to and including the last format specifier in the
> format string. Any data following the last format specifier will not be
> verified. Yet another reason to consider the scanf functions harmful...
>
> Cc: Bert Wesarg
Looks good, thanks.
Reviewed-by: Bert Wesarg
> Signed-off-by: Johan Herland
> ---
> refs.c | 82
> +++--
> t/t6300-for-each-ref.sh | 12
> 2 files changed, 43 insertions(+), 51 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index d17931a..7231f54 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2945,80 +2945,60 @@ struct ref *find_ref_by_name(const struct ref *list,
> const char *name)
> return NULL;
> }
>
> -/*
> - * generate a format suitable for scanf from a ref_rev_parse_rules
> - * rule, that is replace the "%.*s" spec with a "%s" spec
> - */
> -static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
> +int shorten_ref(const char *refname, const char *pattern, char *short_name)
> {
> - char *spec;
> -
> - spec = strstr(rule, "%.*s");
> - if (!spec || strstr(spec + 4, "%.*s"))
> - die("invalid rule in ref_rev_parse_rules: %s", rule);
> -
> - /* copy all until spec */
> - strncpy(scanf_fmt, rule, spec - rule);
> - scanf_fmt[spec - rule] = '\0';
> - /* copy new spec */
> - strcat(scanf_fmt, "%s");
> - /* copy remaining rule */
> - strcat(scanf_fmt, spec + 4);
> -
> - return;
> + /*
> +* pattern must be of the form "[pre]%.*s[post]". Check if refname
> +* starts with "[pre]" and ends with "[post]". If so, write the
> +* middle part into short_name, and return the number of chars
> +* written (not counting the added NUL-terminator). Otherwise,
> +* if refname does not match pattern, return 0.
> +*/
> + size_t pre_len, post_start, post_len, match_len;
> + size_t ref_len = strlen(refname);
> + char *sep = strstr(pattern, "%.*s");
> + if (!sep || strstr(sep + 4, "%.*s"))
> + die("invalid pattern in ref_rev_parse_rules: %s", pattern);
> + pre_len = sep - pattern;
> + post_start = pre_len + 4;
> + post_len = strlen(pattern + post_start);
> + if (pre_len + post_len >= ref_len)
> + return 0; /* refname too short */
> + match_len =
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
On Tue, Sep 18, 2012 at 4:52 AM, Mischa POSLAWSKY wrote: > Junio C Hamano skribis 2012-9-16 22:22 (-0700): > >> Mischa POSLAWSKY writes: >> >> > Subject: [PATCH/RFC] format-patch: force default file prefixes in diff >> > >> > Override user configuration (eg. diff.noprefix) in patches intended for >> > external consumption to match the default prefixes expected by git-am. >> > >> > Signed-off-by: Mischa POSLAWSKY >> > --- >> >> Not all projects expect to see a/ & b/ prefix and these are >> configurable for a reason. Robbing the choice that has been >> supported for quite a long time from them is an unacceptable >> regression. > > My bad, I was assuming format-patch would mostly interact with git am. > >> Why did you think this may be a good idea in the first place? >> >> Perhaps you had configured your diff.noprefix in a wrong >> configuration file? This is primarily per-project choice, and your >> clone of git.git should not have diff.noprefix set, neither your >> $HOME/.gitconfig unless you always work on projects that want >> diff.noprefix. > > Then I'm not using it as intended. For me it's just a personal > preference of how I'd like to review commits (diff/show) so I can easily > copy-paste file names (less essential since my discovery of git jump, > but still). It's not something I'd like to be communicated with any > upstream project (format-patch). > > So it seems I'm asking for a new feature: to be able to configure local > and inter-project diff options differently. In this case I'd be helped > by either format.noprefix=0 or a to be bikeshedded localdiff.noprefix=1. > I don't know about other options though. Does anybody actually want > mnemonicprefix to be sent out as well? I once had the same idea and posted a patch for it: http://article.gmane.org/gmane.comp.version-control.git/146215 The implementation differs though, the pure path without any mnemonicprefix is in its own line 'path '. But my current patch also differs to this old one, in the current one, the path is at the end 'index' line. But I do not need this feature anymore. Bert > > Another solution could be a single option defining behaviour exceptions: > format.diff = normal | textconv | noconfig > Expanding on the existing --(no-)textconv difference in format-patch. > > -- > Mischa -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: Fix semi-working shortcuts for unstage and revert
On Sat, Sep 15, 2012 at 1:36 AM, wrote:
> From: Vitaly _Vi Shukela
>
> Make Ctrl+U for unstaging and Ctrl+J for reverting selection behave
> more like Ctrl+T for adding.
>
> They were working only when one area was focused (diff or commit message),
> now they should work everywhere.
>
> Signed-off-by: Vitaly _Vi Shukela
> ---
> Sending the patch the third time (haven't got any replies to previous two
> attempts).
For what its worth:
Acked-by: Bert Wesarg
But unless Pat reacts this is useless.
Bert
>
> git-gui/git-gui.sh |4
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index ba4e5c1..6618016 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -3710,6 +3710,8 @@ bind $ui_diff <$M1B-Key-v> {break}
> bind $ui_diff <$M1B-Key-V> {break}
> bind $ui_diff <$M1B-Key-a> {%W tag add sel 0.0 end;break}
> bind $ui_diff <$M1B-Key-A> {%W tag add sel 0.0 end;break}
> +bind $ui_diff <$M1B-Key-j> {do_revert_selection;break}
> +bind $ui_diff <$M1B-Key-J> {do_revert_selection;break}
> bind $ui_diff {catch {%W yview scroll -1 units};break}
> bind $ui_diff{catch {%W yview scroll 1 units};break}
> bind $ui_diff{catch {%W xview scroll -1 units};break}
> @@ -3742,6 +3744,8 @@ bind . <$M1B-Key-s> do_signoff
> bind . <$M1B-Key-S> do_signoff
> bind . <$M1B-Key-t> do_add_selection
> bind . <$M1B-Key-T> do_add_selection
> +bind . <$M1B-Key-u> do_unstage_selection
> +bind . <$M1B-Key-U> do_unstage_selection
> bind . <$M1B-Key-j> do_revert_selection
> bind . <$M1B-Key-J> do_revert_selection
> bind . <$M1B-Key-i> do_add_all
> --
> 1.7.8.5
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
