| Message ID | 20200910170159.1278781-1-gitster@pobox.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | quote_path() clean-ups | expand |
On 2020-09-10 at 17:01:52, Junio C Hamano wrote: > Here is an update, after seeing Peff's review. > > The overall structure of the series stays the same. > > * The second patch lost an incorrect use of 'flags' parameter down > to quote_c_style_counted() from quote_path(). In the function > declaration of quote_path(), the flags parameter is now named. > > * The third patch that moves the "optionally quote path with SP in > it" logic to quote_path() is essentialy unchanged. > > * But the fourth patch rewrites the implementation to avoid > shifting the output bytes with insertstr(). > > * The fifth patch (formerly the fourth) that teachs "status --short" > to consistently quote hasn't changed, except that we test also > for the ignored paths. > > * The last two patches are unchanged. They are optional clean-ups > that are not required. > > We might want to add a bit more tests for: > > - how conflicted "funny" paths are shown. > > - how doubly funny paths (those that quote_c_style_counted() needs > to use backslash quoting *and* that contain SP) are shown. > > but I'd say that is outside the scope of this round. Seeing what is > done in t3300, the latter test cannot be written portably as far as > I can tell. I took a look and this seems like a great improvement. Thanks for picking up where my patch left off.
Here is an update, after seeing Peff's review. The overall structure of the series stays the same. * The second patch lost an incorrect use of 'flags' parameter down to quote_c_style_counted() from quote_path(). In the function declaration of quote_path(), the flags parameter is now named. * The third patch that moves the "optionally quote path with SP in it" logic to quote_path() is essentialy unchanged. * But the fourth patch rewrites the implementation to avoid shifting the output bytes with insertstr(). * The fifth patch (formerly the fourth) that teachs "status --short" to consistently quote hasn't changed, except that we test also for the ignored paths. * The last two patches are unchanged. They are optional clean-ups that are not required. We might want to add a bit more tests for: - how conflicted "funny" paths are shown. - how doubly funny paths (those that quote_c_style_counted() needs to use backslash quoting *and* that contain SP) are shown. but I'd say that is outside the scope of this round. Seeing what is done in t3300, the latter test cannot be written portably as far as I can tell. Junio C Hamano (7): quote_path: rename quote_path_relative() to quote_path() quote_path: give flags parameter to quote_path() quote_path: optionally allow quoting a path with SP in it quote_path: code clarification wt-status: consistently quote paths in "status --short" output quote: rename misnamed sq_lookup[] to cq_lookup[] quote: turn 'nodq' parameter into a set of flags builtin/clean.c | 22 +++++++++++----------- builtin/grep.c | 2 +- diff.c | 8 ++++---- quote.c | 47 +++++++++++++++++++++++++++++++---------------- quote.h | 11 +++++++---- t/t7508-status.sh | 27 +++++++++++++++++++++++++++ wt-status.c | 37 +++++++++++++------------------------ 7 files changed, 94 insertions(+), 60 deletions(-) Range-diff against v1: 1: a9306429f4 = 1: a9306429f4 quote_path: rename quote_path_relative() to quote_path() 2: c2784628a4 ! 2: 179d12d16f quote_path: give flags parameter to quote_path() @@ quote.c: void write_name_quoted_relative(const char *name, const char *prefix, { struct strbuf sb = STRBUF_INIT; const char *rel = relative_path(in, prefix, &sb); - strbuf_reset(out); -- quote_c_style_counted(rel, strlen(rel), out, NULL, 0); -+ quote_c_style_counted(rel, strlen(rel), out, NULL, flags); - strbuf_release(&sb); - - return out->buf; ## quote.h ## @@ quote.h: void write_name_quoted_relative(const char *name, const char *prefix, @@ quote.h: void write_name_quoted_relative(const char *name, const char *prefix, /* quote path as relative to the given prefix */ -char *quote_path(const char *in, const char *prefix, struct strbuf *out); -+char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned); ++char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags); /* quoting as a string literal for other languages */ void perl_quote_buf(struct strbuf *sb, const char *src); 3: 640673148e ! 3: d566c38d2f quote_path: optionally allow quoting a path with SP in it @@ Commit message ## quote.c ## @@ quote.c: char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne - struct strbuf sb = STRBUF_INIT; - const char *rel = relative_path(in, prefix, &sb); - strbuf_reset(out); -- quote_c_style_counted(rel, strlen(rel), out, NULL, flags); -+ quote_c_style_counted(rel, strlen(rel), out, NULL, 0); + quote_c_style_counted(rel, strlen(rel), out, NULL, 0); strbuf_release(&sb); + if ((flags & QUOTE_PATH_QUOTE_SP) && @@ quote.h @@ quote.h: void write_name_quoted_relative(const char *name, const char *prefix, /* quote path as relative to the given prefix */ - char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned); + char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags); +#define QUOTE_PATH_QUOTE_SP 01 /* quoting as a string literal for other languages */ -: ---------- > 4: f3769b7cf4 quote_path: code clarification 4: ac9a0c4a33 ! 5: 498bed71e4 wt-status: consistently quote paths in "status --short" output @@ Commit message output, but untracked, ignored, and unmerged paths weren't. The test was stolen from a patch to fix output for the 'untracked' - paths by brian m. carlson. + paths by brian m. carlson, with similar tests added for 'ignored' + ones. Signed-off-by: Junio C Hamano <gitster@pobox.com> @@ t/t7508-status.sh: test_expect_success 'status -s without relative paths' ' + test_when_finished "git rm --cached \"file with spaces\"; rm -f file*" && + >"file with spaces" && + >"file with spaces 2" && ++ >"expect with spaces" && + git add "file with spaces" && ++ + git status -s >output && -+ test_cmp expect output ++ test_cmp expect output && + ++ git status -s --ignored >output && ++ grep "^!! \"expect with spaces\"$" output && ++ grep -v "^!! " output >output-wo-ignored && ++ test_cmp expect output-wo-ignored +' + test_expect_success 'dry-run of partial commit excluding new file in index' ' 5: 57c6294695 = 6: e74186bbd7 quote: rename misnamed sq_lookup[] to cq_lookup[] 6: 480152cfe4 ! 7: d87d7dd561 quote: turn 'nodq' parameter into a set of flags @@ quote.c: static size_t quote_c_style_counted(const char *name, ssize_t maxlen, if (!nodq) strbuf_addch(sb, '"'); } else { +@@ quote.c: char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne + */ + if (force_dq) + strbuf_addch(out, '"'); +- quote_c_style_counted(rel, strlen(rel), out, NULL, !!force_dq); ++ quote_c_style_counted(rel, strlen(rel), out, NULL, ++ force_dq ? CQUOTE_NODQ : 0); + if (force_dq) + strbuf_addch(out, '"'); + strbuf_release(&sb); ## quote.h ## @@ quote.h: struct strvec;