[v2,1/3] grep: perform some minor code and comment cleanups

Commit Message

Dragan Simic March 24, 2024, 5:51 p.m. UTC

Move some variable definitions around, and reflow one comment block, to
make the code a bit neater after spotting those slightly unpolished areas.
There are no functional changes to the source code.

Signed-off-by: Dragan Simic <[e-mail]>
---

Notes:
    Changes in v2:
        - No changes were introduced
    
    This patch is salvaged from my earlier series, [1] for which it has been
    concluded to be not acceptable for merging, because of possible issues
    with various git scripts. [2]
    
    Compared to the version in the earlies series, there are no changes in
    this version.  As expected and as already discussed, patches like this one
    inevitably raise a few eyebrows. [3][4][5]
    
    [1] https://lore.kernel.org/git/[e-mail]/T/#u
    [2] https://lore.kernel.org/git/[e-mail]/
    [3] https://lore.kernel.org/git/[e-mail]/
    [4] https://lore.kernel.org/git/xmqqjzlzt61d.fsf@gitster.g/#t
    [5] https://lore.kernel.org/git/[e-mail]/

 builtin/grep.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

Comments

Junio C Hamano March 25, 2024, 9:03 p.m. UTC | #1

Dragan Simic <[e-mail]> writes:

> Move some variable definitions around, and reflow one comment block, to
> make the code a bit neater after spotting those slightly unpolished areas.
> There are no functional changes to the source code.

It cannot be objectively better than the original as "neater" and
'unpolished" are fairly subjective.  Let's leave this out and take
the two "obviously good" documentation improvements for now.

Thanks.

Dragan Simic March 25, 2024, 9:17 p.m. UTC | #2

On 2024-03-25 22:03, Junio C Hamano wrote:
> Dragan Simic <[e-mail]> writes:
> 
>> Move some variable definitions around, and reflow one comment block, 
>> to
>> make the code a bit neater after spotting those slightly unpolished 
>> areas.
>> There are no functional changes to the source code.
> 
> It cannot be objectively better than the original as "neater" and
> 'unpolished" are fairly subjective.  Let's leave this out and take
> the two "obviously good" documentation improvements for now.

That's fine with me.  This "experiment", so to speak, has also taught me
not to pay attention to making the code "neater".

Junio C Hamano March 25, 2024, 9:28 p.m. UTC | #3

Dragan Simic <[e-mail]> writes:

> That's fine with me.  This "experiment", so to speak, has also taught me
> not to pay attention to making the code "neater".

True.  You should not give too much weight in your sense of what is
"neater".  Spending cycles on something you can measure is more
productive.

Thanks.

Patch

diff --git a/builtin/grep.c b/builtin/grep.c
index 982bcfc4b1df..af89c8b5cb19 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -623,13 +623,13 @@  static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 		     struct tree_desc *tree, struct strbuf *base, int tn_len,
 		     int check_attr)
 {
-	struct repository *repo = opt->repo;
-	int hit = 0;
+	int hit = 0, name_base_len = 0;
+	int old_baselen = base->len;
 	enum interesting match = entry_not_interesting;
+	struct repository *repo = opt->repo;
 	struct name_entry entry;
-	int old_baselen = base->len;
 	struct strbuf name = STRBUF_INIT;
-	int name_base_len = 0;
+
 	if (repo->submodule_prefix) {
 		strbuf_addstr(&name, repo->submodule_prefix);
 		name_base_len = name.len;
@@ -890,19 +890,15 @@  static int pattern_callback(const struct option *opt, const char *arg,
 
 int cmd_grep(int argc, const char **argv, const char *prefix)
 {
-	int hit = 0;
+	int hit = 0, seen_dashdash = 0, use_index = 1;
 	int cached = 0, untracked = 0, opt_exclude = -1;
-	int seen_dashdash = 0;
 	int external_grep_allowed__ignored;
+	int i, dummy, allow_revs;
 	const char *show_in_pager = NULL, *default_pager = "dummy";
 	struct grep_opt opt;
 	struct object_array list = OBJECT_ARRAY_INIT;
 	struct pathspec pathspec;
 	struct string_list path_list = STRING_LIST_INIT_DUP;
-	int i;
-	int dummy;
-	int use_index = 1;
-	int allow_revs;
 
 	struct option options[] = {
 		OPT_BOOL(0, "cached", &cached,
@@ -1059,9 +1055,8 @@  int cmd_grep(int argc, const char **argv, const char *prefix)
 		recurse_submodules = 0;
 
 	/*
-	 * skip a -- separator; we know it cannot be
-	 * separating revisions from pathnames if
-	 * we haven't even had any patterns yet
+	 * skip a -- separator; we know it cannot be separating revisions
+	 * from pathnames if we haven't even had any patterns yet
 	 */
 	if (argc > 0 && !opt.pattern_list && !strcmp(argv[0], "--")) {
 		argv++;