Re: [PATCH 4/4] branch: add '--show-description' option

2018-01-02 Thread SZEDER Gábor 

On Tue, Jan 2, 2018 at 10:32 AM, Johannes Sixt <j...@kdbg.org> wrote:
> > Which makes me wonder: Why would --show-description have to error out
> > silently? This is not 'git config' after all.

> I don't have a strong opinion about this, and certainly wouldn't mind
> adding an error message instead.

And it would look like this:

  -- >8 --

Subject: [PATCH 4/4] branch: add '--show-description' option

'git branch' has an option to edit a branch's description, but lacks
the option to show that description.

Therefore, add a new '--show-description' option to do just that, as a
more user-friendly alternative to 'git config --get
branch.$branchname.description':

  - it's shorter to type (both in the number of characters and the
number of TABs if using completion),
  - works on the current branch without explicitly naming it,
  - hides the implementation detail that branch descriptions are
stored in the config file, and
  - errors out with a proper error message when the given branch
doesn't exist or has no description.

Signed-off-by: SZEDER Gábor <szeder@gmail.com>
---
 Documentation/git-branch.txt   |  6 +-
 builtin/branch.c   | 39 +++---
 contrib/completion/git-completion.bash |  4 ++--
 t/t3200-branch.sh  | 21 ++
 4 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index b3084c99c..e05c9e193 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -20,7 +20,7 @@ SYNOPSIS
 'git branch' (-m | -M) [] 
 'git branch' (-c | -C) [] 
 'git branch' (-d | -D) [-r] ...
-'git branch' --edit-description []
+'git branch' (--edit-description | --show-description) []
 
 DESCRIPTION
 ---
@@ -226,6 +226,10 @@ start-point is either a local or remote-tracking branch.
`request-pull`, and `merge` (if enabled)). Multi-line explanations
may be used.
 
+--show-description::
+   Show the description of the branch previously set using
+   `--edit-description`.
+
 --contains []::
Only list branches which contain the specified commit (HEAD
if not specified). Implies `--list`.
diff --git a/builtin/branch.c b/builtin/branch.c
index 32531aa44..748a1a575 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -573,7 +573,7 @@ static int edit_branch_description(const char *branch_name)
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
-   int reflog = 0, edit_description = 0;
+   int reflog = 0, edit_description = 0, show_description = 0;
int quiet = 0, unset_upstream = 0;
const char *new_upstream = NULL;
enum branch_track track;
@@ -615,6 +615,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPT_BOOL('l', "create-reflog", , N_("create the branch's 
reflog")),
OPT_BOOL(0, "edit-description", _description,
 N_("edit the description for the branch")),
+   OPT_BOOL(0, "show-description", _description,
+N_("show the description for the branch")),
OPT__FORCE(, N_("force creation, move/rename, deletion")),
OPT_MERGED(, N_("print only branches that are merged")),
OPT_NO_MERGED(, N_("print only branches that are not 
merged")),
@@ -654,7 +656,9 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 0);
 
-   if (!delete && !rename && !copy && !edit_description && !new_upstream 
&& !unset_upstream && argc == 0)
+   if (!delete && !rename && !copy &&
+   !edit_description && !show_description &&
+   !new_upstream && !unset_upstream && argc == 0)
list = 1;
 
if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || 
filter.points_at.nr ||
@@ -662,7 +666,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
list = 1;
 
if (!!delete + !!rename + !!copy + !!new_upstream +
-   list + unset_upstream + edit_description > 1)
+   list + unset_upstream + edit_description + show_description > 1)
usage_with_options(builtin_branch_usage, options);
 
if (filter.abbrev == -1)
@@ -737,6 +741,35 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 
if (edit_branch_description(branch_name))
return 1;
+   } else if (show_description) {
+   const char *branch_name;
+

Re: [PATCH 4/4] branch: add '--show-description' option

2018-01-02 Thread SZEDER Gábor 
On Tue, Jan 2, 2018 at 10:32 AM, Johannes Sixt  wrote:
> Am 01.01.2018 um 23:54 schrieb SZEDER Gábor:

>>- errors out with a proper error message when the given branch
>>  doesn't exist (but exits quietly with an error code when the
>>  branch does exit but has no description, just like the 'git config'
>>  query does).
>
>
>> +test_expect_success '--show-description with no description errors
>> quietly' '
>> +   git config --unset branch.master.description &&
>> +   test_must_fail git branch --show-description >actual 2>actual.err
>> &&
>> +   test_must_be_empty actual &&
>> +   test_must_be_empty actual.err
>> +'
>
>
> Checking the exact contents of stderr typically fails when tests are run
> under -x. Perhaps
>
> test_i18ngrep ! "fatal: " actual.err &&"
> test_i18ngrep ! "error: " actual.err &&
> test_i18ngrep ! "warning: " actual.err
>
> Which makes me wonder: Why would --show-description have to error out
> silently? This is not 'git config' after all.

I figured it would be beneficial if it were a drop-in replacement for
the original 'git config' query.

I don't have a strong opinion about this, and certainly wouldn't mind
adding an error message instead.

Gábor

Re: [PATCH 4/4] branch: add '--show-description' option

2018-01-02 Thread SZEDER Gábor 
On Tue, Jan 2, 2018 at 6:17 AM, Eric Sunshine  wrote:
> On Mon, Jan 1, 2018 at 5:54 PM, SZEDER Gábor  wrote:

> s/exit/exist/

Thanks.

>> query does).
>>
>> Signed-off-by: SZEDER Gábor 
>> ---
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> @@ -737,6 +741,35 @@ int cmd_branch(int argc, const char **argv, const char 
>> *prefix)
>> +   } else if (show_description) {
>> +   [...]
>> +   if (!argc) {
>> +   if (filter.detached)
>> +   die(_("cannot show description on detached 
>> HEAD"));
>> +   branch_name = head;
>> +   } else if (argc == 1)
>> +   branch_name = argv[0];
>> +   else
>> +   die(_("cannot show description of more than one 
>> branch"));
>
> Aside from paralleling the single branch accepted by
> --edit-description, why this limitation? (Just curious; I don't feel
> strongly one way or the other.)

It's not just '--edit-description', most other options won't accept
multiple branches either.  As far as I can tell only deleting branches
can deal with multiple branches in one go.

Furthermore, branch descriptions are likely more lines long, so we
can't just dump them one after the other, but we we would have to
separate the descriptions of different branches in the output.
Considering that 'git branch' mostly works only with a single branch at
a time, I didn't feel the need to do so.

Gábor

Re: [PATCH 4/4] branch: add '--show-description' option

2018-01-02 Thread Johannes Sixt 

Am 01.01.2018 um 23:54 schrieb SZEDER Gábor:

'git branch' has an option to edit a branch's description, but lacks
the option to show that description.

Therefore, add a new '--show-description' option to do just that, as a
more user-friendly alternative to 'git config --get
branch.$branchname.description':

   - it's shorter to type (both in the number of characters and the
 number of TABs if using completion),
   - works on the current branch without explicitly naming it,
   - hides the implementation detail that branch descriptions are
 stored in the config file, and
   - errors out with a proper error message when the given branch
 doesn't exist (but exits quietly with an error code when the
 branch does exit but has no description, just like the 'git config'
 query does).



+test_expect_success '--show-description with no description errors quietly' '
+   git config --unset branch.master.description &&
+   test_must_fail git branch --show-description >actual 2>actual.err &&
+   test_must_be_empty actual &&
+   test_must_be_empty actual.err
+'


Checking the exact contents of stderr typically fails when tests are run 
under -x. Perhaps


test_i18ngrep ! "fatal: " actual.err &&"
test_i18ngrep ! "error: " actual.err &&
test_i18ngrep ! "warning: " actual.err

Which makes me wonder: Why would --show-description have to error out 
silently? This is not 'git config' after all.


-- Hannes

Re: [PATCH 4/4] branch: add '--show-description' option

2018-01-01 Thread Eric Sunshine 
On Mon, Jan 1, 2018 at 5:54 PM, SZEDER Gábor  wrote:
> 'git branch' has an option to edit a branch's description, but lacks
> the option to show that description.
>
> Therefore, add a new '--show-description' option to do just that, as a
> more user-friendly alternative to 'git config --get
> branch.$branchname.description':
>   [...]
>   - errors out with a proper error message when the given branch
> doesn't exist (but exits quietly with an error code when the
> branch does exit but has no description, just like the 'git config'

s/exit/exist/

> query does).
>
> Signed-off-by: SZEDER Gábor 
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> @@ -737,6 +741,35 @@ int cmd_branch(int argc, const char **argv, const char 
> *prefix)
> +   } else if (show_description) {
> +   [...]
> +   if (!argc) {
> +   if (filter.detached)
> +   die(_("cannot show description on detached 
> HEAD"));
> +   branch_name = head;
> +   } else if (argc == 1)
> +   branch_name = argv[0];
> +   else
> +   die(_("cannot show description of more than one 
> branch"));

Aside from paralleling the single branch accepted by
--edit-description, why this limitation? (Just curious; I don't feel
strongly one way or the other.)

[PATCH 4/4] branch: add '--show-description' option

2018-01-01 Thread SZEDER Gábor 
'git branch' has an option to edit a branch's description, but lacks
the option to show that description.

Therefore, add a new '--show-description' option to do just that, as a
more user-friendly alternative to 'git config --get
branch.$branchname.description':

  - it's shorter to type (both in the number of characters and the
number of TABs if using completion),
  - works on the current branch without explicitly naming it,
  - hides the implementation detail that branch descriptions are
stored in the config file, and
  - errors out with a proper error message when the given branch
doesn't exist (but exits quietly with an error code when the
branch does exit but has no description, just like the 'git config'
query does).

Signed-off-by: SZEDER Gábor 
---
 Documentation/git-branch.txt   |  6 +-
 builtin/branch.c   | 39 +++---
 contrib/completion/git-completion.bash |  4 ++--
 t/t3200-branch.sh  | 23 
 4 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index b3084c99c..e05c9e193 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -20,7 +20,7 @@ SYNOPSIS
 'git branch' (-m | -M) [] 
 'git branch' (-c | -C) [] 
 'git branch' (-d | -D) [-r] ...
-'git branch' --edit-description []
+'git branch' (--edit-description | --show-description) []
 
 DESCRIPTION
 ---
@@ -226,6 +226,10 @@ start-point is either a local or remote-tracking branch.
`request-pull`, and `merge` (if enabled)). Multi-line explanations
may be used.
 
+--show-description::
+   Show the description of the branch previously set using
+   `--edit-description`.
+
 --contains []::
Only list branches which contain the specified commit (HEAD
if not specified). Implies `--list`.
diff --git a/builtin/branch.c b/builtin/branch.c
index 32531aa44..f2f6614e2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -573,7 +573,7 @@ static int edit_branch_description(const char *branch_name)
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
-   int reflog = 0, edit_description = 0;
+   int reflog = 0, edit_description = 0, show_description = 0;
int quiet = 0, unset_upstream = 0;
const char *new_upstream = NULL;
enum branch_track track;
@@ -615,6 +615,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPT_BOOL('l', "create-reflog", , N_("create the branch's 
reflog")),
OPT_BOOL(0, "edit-description", _description,
 N_("edit the description for the branch")),
+   OPT_BOOL(0, "show-description", _description,
+N_("show the description for the branch")),
OPT__FORCE(, N_("force creation, move/rename, deletion")),
OPT_MERGED(, N_("print only branches that are merged")),
OPT_NO_MERGED(, N_("print only branches that are not 
merged")),
@@ -654,7 +656,9 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 0);
 
-   if (!delete && !rename && !copy && !edit_description && !new_upstream 
&& !unset_upstream && argc == 0)
+   if (!delete && !rename && !copy &&
+   !edit_description && !show_description &&
+   !new_upstream && !unset_upstream && argc == 0)
list = 1;
 
if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || 
filter.points_at.nr ||
@@ -662,7 +666,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
list = 1;
 
if (!!delete + !!rename + !!copy + !!new_upstream +
-   list + unset_upstream + edit_description > 1)
+   list + unset_upstream + edit_description + show_description > 1)
usage_with_options(builtin_branch_usage, options);
 
if (filter.abbrev == -1)
@@ -737,6 +741,35 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 
if (edit_branch_description(branch_name))
return 1;
+   } else if (show_description) {
+   const char *branch_name;
+   struct strbuf buf = STRBUF_INIT;
+   char *description = NULL;
+
+   if (!argc) {
+   if (filter.detached)
+   die(_("cannot show description on detached 
HEAD"));
+   branch_name = head;
+   } else if (argc == 1)
+   branch_name = argv[0];
+   else
+   die(_("cannot show description of more than one 
branch"));
+
+   strbuf_addf(, "refs/heads/%s",