Kaartic Sivaraam [off-list ref] writes:
quoted
I personally do not find these new blank lines are necessary, and
this change wastes vertical screen real estate which is a limited
resource, but that may be just me. I on the other hand do not think
the result of this patch is overly worse than the status quo, either.
I thought it's not good to trade-off readability for vertical space as
the ultimate aim of the commit template (at least to me) is to convey
information to the user about the commit that he's going to make. For
which, I thought it made more sense to improve it's readability by
adding new lines between different sections rather than constrain the
output within a few lines.
You have to be careful when making a trade-off argument. It depends
on how familiar you already are with the presentation. Those who
are/got used to the order of things that come, they will know there
is extra information when the block of lines are longer than usual
without reading every character and then their eyes are guided to
read what is extra, without having to waste precious screen real
estate. Nobody will _stay_ a new user who is not yet familiar with
the everyday output.
quoted
If we were to go with this sparser output, I think we also should
give an extra blank line before and after the "HEAD detached from
cafebabe" message you would see:
$ git checkout HEAD^0
$ git commit --allow-empty -o
or "On branch blah" if you are on a branch. I think your change
adds a blank before, but it does not have a separation before
"Changes not staged for commit" line.
I actually didn't think of modifying that in order to keep it in line
with the output of `git status`.
I was (and still am) assuming that if we make this change to "git
commit", we should make matching change to "git status" as a given.
Further, to me, adding *this* new line
before the "Changes not staged for commit" (or something in it's place)
seems to be wasting some vertical space ...
I think it is in line with your original reasoning why you wanted
these extra blank lines to separate blocks of different kinds of
information:
- "Please do this" instruction at the beginning
- Make sure you know the default is --only, not --include
- By the way you are committing for that person, not you
- This change is being committed on that branch
- Here are the changes that are already in the index
- Here are the changes that are not in the index
- Here are untracked files
Lack of a blank between the fourth block and the fifth block [*1*]
makes it somewhat inconsistent, doesn't it?
[Footnote]
*1* Yes, we should think about removing the optional second block,
as I think that it outlived its usefulness; if we are to do so,
these become the third and the fourth blocks.
On Tue, 2017-06-27 at 10:56 -0700, Junio C Hamano wrote:
Kaartic Sivaraam [off-list ref] writes:
quoted
I thought it's not good to trade-off readability for vertical space
as
the ultimate aim of the commit template (at least to me) is to
convey
information to the user about the commit that he's going to make.
For
which, I thought it made more sense to improve it's readability by
adding new lines between different sections rather than constrain
the
output within a few lines.
You have to be careful when making a trade-off argument. It depends
on how familiar you already are with the presentation. Those who
are/got used to the order of things that come, they will know there
is extra information when the block of lines are longer than usual
without reading every character and then their eyes are guided to
read what is extra, without having to waste precious screen real
estate. Nobody will _stay_ a new user who is not yet familiar with
the everyday output.
You're right. I didn't consider the fact that experienced users would
be affected as a result of this change, sorry about that. I thought,
making this change would help the new users who would possibly find the
commit template to be congested and let experienced users to get
accustomed to this new output format. I thought this change would be a
win-win (at least after people get accustomed to the new formatting).
In case screen real estate is considered more important here, no
issues. I'll drop that part of the change, happily.
quoted
I actually didn't think of modifying that in order to keep it in
line
with the output of `git status`.
I was (and still am) assuming that if we make this change to "git
commit", we should make matching change to "git status" as a given.
I get it now. In that case, I don't think making the change would be a
good choice for the following reasons,
* I think vertical spacing matters more in the output printed to a
console.
* I myself find it odd to add a new line below the branch
information possibly because I'm too accustomed to it's current
output.
I tried adding the new line, it seemed to be too spacious. It might be
just me in this case.
quoted
Further, to me, adding *this* new line
before the "Changes not staged for commit" (or something in it's
place)
seems to be wasting some vertical space ...
I think it is in line with your original reasoning why you wanted
these extra blank lines to separate blocks of different kinds of
information:
- "Please do this" instruction at the beginning
- Make sure you know the default is --only, not --include
- By the way you are committing for that person, not you
- This change is being committed on that branch
- Here are the changes that are already in the index
- Here are the changes that are not in the index
- Here are untracked files
Lack of a blank between the fourth block and the fifth block [*1*]
makes it somewhat inconsistent, doesn't it?
It does, for the given set of blocks. I didn't find it inconsistent as
I thought the separate blocks as follows,
- "Please do this" instruction at the beginning
- Make sure you know the default is --only, not --include
- By the way you are committing for that person, not you
- Status of repository (git status)
[Footnote]
*1* Yes, we should think about removing the optional second block,
as I think that it outlived its usefulness; if we are to do so,
these become the third and the fourth blocks.
If I interpreted your previous email correctly, I thought we were doing
it!
I'll send a "typical" patch as a follow-up of this mail.
--
Regards,
Kaartic Sivaraam [off-list ref]
The commit template adds the optional parts without
a new line to distinguish them. This results in
difficulty in interpreting it's content. Add new
lines to separate the distinct parts of the template.
The warning about usage of 'explicit paths without
any corresponding options' has outlived it's purpose of
warning users about the usage '--only' as default rather
than '--include'. Remove it.
Signed-off-by: Kaartic Sivaraam <redacted>
---
builtin/commit.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 8d1cac062..22d17e6f2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -139,7 +139,6 @@ static enum commit_whence whence;
static int sequencer_in_use;
static int use_editor = 1, include_status = 1;
static int show_ignored_in_status, have_option_m;
-static const char *only_include_assumed;
static struct strbuf message = STRBUF_INIT;
static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -841,9 +840,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
"with '%c' will be kept; you may remove them"
" yourself if you want to.\n"
"An empty message aborts the commit.\n"), comment_line_char);
- if (only_include_assumed)
- status_printf_ln(s, GIT_COLOR_NORMAL,
- "%s", only_include_assumed);
/*
* These should never fail because they come from our own
@@ -877,8 +873,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
(int)(ci.name_end - ci.name_begin), ci.name_begin,
(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);
- if (ident_shown)
- status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+ status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); /* Add new line for clarity */
saved_color_setting = s->use_color;
s->use_color = 0;
@@ -1208,8 +1203,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
if (argc == 0 && (also || (only && !amend && !allow_empty)))
die(_("No paths with --include/--only does not make sense."));
- if (argc > 0 && !also && !only)
- only_include_assumed = _("Explicit paths specified without -i or -o; assuming --only paths...");
if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
else if (!strcmp(cleanup_arg, "verbatim"))--
2.11.0
I might have been ignorant about something about git in my reply in the
previous email (found below). In that case, please enlighten me.
On Wed, 2017-06-28 at 18:34 +0530, Kaartic Sivaraam wrote:
On Tue, 2017-06-27 at 10:56 -0700, Junio C Hamano wrote:
quoted
Kaartic Sivaraam [off-list ref] writes:
quoted
I thought it's not good to trade-off readability for vertical
space
as
the ultimate aim of the commit template (at least to me) is to
convey
information to the user about the commit that he's going to make.
For
which, I thought it made more sense to improve it's readability
by
adding new lines between different sections rather than constrain
the
output within a few lines.
You have to be careful when making a trade-off argument. It
depends
on how familiar you already are with the presentation. Those who
are/got used to the order of things that come, they will know there
is extra information when the block of lines are longer than usual
without reading every character and then their eyes are guided to
read what is extra, without having to waste precious screen real
estate. Nobody will _stay_ a new user who is not yet familiar with
the everyday output.
You're right. I didn't consider the fact that experienced users would
be affected as a result of this change, sorry about that. I thought,
making this change would help the new users who would possibly find
the
commit template to be congested and let experienced users to get
accustomed to this new output format. I thought this change would be
a
win-win (at least after people get accustomed to the new
formatting).
In case screen real estate is considered more important here, no
issues. I'll drop that part of the change, happily.
quoted
quoted
I actually didn't think of modifying that in order to keep it in
line
with the output of `git status`.
I was (and still am) assuming that if we make this change to "git
commit", we should make matching change to "git status" as a given.
I get it now. In that case, I don't think making the change would be
a
good choice for the following reasons,
* I think vertical spacing matters more in the output printed to
a
console.
* I myself find it odd to add a new line below the branch
information possibly because I'm too accustomed to it's current
output.
I tried adding the new line, it seemed to be too spacious. It might
be
just me in this case.
quoted
quoted
Further, to me, adding *this* new line
before the "Changes not staged for commit" (or something in it's
place)
seems to be wasting some vertical space ...
I think it is in line with your original reasoning why you wanted
these extra blank lines to separate blocks of different kinds of
information:
- "Please do this" instruction at the beginning
- Make sure you know the default is --only, not --include
- By the way you are committing for that person, not you
- This change is being committed on that branch
- Here are the changes that are already in the index
- Here are the changes that are not in the index
- Here are untracked files
Lack of a blank between the fourth block and the fifth block [*1*]
makes it somewhat inconsistent, doesn't it?
It does, for the given set of blocks. I didn't find it inconsistent
as
I thought the separate blocks as follows,
- "Please do this" instruction at the beginning
- Make sure you know the default is --only, not --include
- By the way you are committing for that person, not you
- Status of repository (git status)
quoted
[Footnote]
*1* Yes, we should think about removing the optional second block,
as I think that it outlived its usefulness; if we are to do so,
these become the third and the fourth blocks.
If I interpreted your previous email correctly, I thought we were
doing
it!
I'll send a "typical" patch as a follow-up of this mail.