On Mon, Dec 05 2022, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>
> The strbuf_parent_directory() method was added as a static method in
> contrib/scalar by d0feac4e8c0 (scalar: 'register' sets recommended
> config and starts maintenance, 2021-12-03) and then removed in
> 65f6a9eb0b9 (scalar: constrain enlistment search, 2022-08-18), but now
> there is a need for a similar method in the bundle URI feature.
>
> Re-add the method, this time in strbuf.c, but with a new name:
> strbuf_strip_file_from_path(). The method requirements are slightly
> modified to allow a trailing slash, in which case nothing is done, which
> makes the name change valuable.
>
> Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
> ---
> strbuf.c | 6 ++++++
> strbuf.h | 11 +++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/strbuf.c b/strbuf.c
> index 0890b1405c5..c383f41a3c5 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -1200,3 +1200,9 @@ int strbuf_edit_interactively(struct strbuf *buffer, const char *path,
> free(path2);
> return res;
> }
> +
> +void strbuf_strip_file_from_path(struct strbuf *sb)
> +{
> + char *path_sep = find_last_dir_sep(sb->buf);
> + strbuf_setlen(sb, path_sep ? path_sep - sb->buf + 1 : 0);
> +}
> diff --git a/strbuf.h b/strbuf.h
> index 76965a17d44..f6dbb9681ee 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -664,6 +664,17 @@ int launch_sequence_editor(const char *path, struct strbuf *buffer,
> int strbuf_edit_interactively(struct strbuf *buffer, const char *path,
> const char *const *env);
>
> +/*
> + * Remove the filename from the provided path string. If the path
> + * contains a trailing separator, then the path is considered a directory
> + * and nothing is modified.
> + *
> + * Examples:
> + * - "/path/to/file" -> "/path/to/"
> + * - "/path/to/dir/" -> "/path/to/dir/"
> + */
> +void strbuf_strip_file_from_path(struct strbuf *sb);
> +
> void strbuf_add_lines(struct strbuf *sb,
> const char *prefix,
> const char *buf,
Re your reply in
https://lore.kernel.org/git/0980dcd4-30eb-4ef4-9369-279abe5ca5a2@xxxxxxxxxx/
I still don't get how this is different from a 1-byte change to
strbuf_trim_trailing_dir_sep(), and if it isn't I think it's confusing
API design to have two very different ways to return the same data.
There you said "The difference is all about whether or not we start with
a slash _and_ no other slash appears in the path.".
But I can't find a case where there's any difference between the two. I
tried this ad-hoc test on top:
diff --git a/help.c b/help.c
index f1e090a4428..b0866b01439 100644
--- a/help.c
+++ b/help.c
@@ -765,6 +765,16 @@ int cmd_version(int argc, const char **argv, const char *prefix)
"also print build options"),
OPT_END()
};
+ struct strbuf sb1 = STRBUF_INIT;
+ struct strbuf sb2 = STRBUF_INIT;
+
+ if (getenv("STR")) {
+ strbuf_addstr(&sb1, getenv("STR"));
+ strbuf_addstr(&sb2, getenv("STR"));
+ strbuf_strip_file_from_path(&sb1);
+ strbuf_trim_trailing_not_dir_sep(&sb2);
+ fprintf(stderr, "%s: %s | %s\n", strcmp(sb1.buf, sb2.buf) ? "NEQ" : "EQ", sb1.buf, sb2.buf);
+ }
argc = parse_options(argc, argv, prefix, options, usage, 0);
diff --git a/strbuf.c b/strbuf.c
index c383f41a3c5..f75d94556fc 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -114,13 +114,23 @@ void strbuf_rtrim(struct strbuf *sb)
sb->buf[sb->len] = '\0';
}
-void strbuf_trim_trailing_dir_sep(struct strbuf *sb)
+static void strbuf_trim_trailing_dir_sep_1(struct strbuf *sb, int flip)
{
- while (sb->len > 0 && is_dir_sep((unsigned char)sb->buf[sb->len - 1]))
+ while (sb->len > 0 && is_dir_sep((unsigned char)sb->buf[sb->len - 1]) - flip)
sb->len--;
sb->buf[sb->len] = '\0';
}
+void strbuf_trim_trailing_dir_sep(struct strbuf *sb)
+{
+ strbuf_trim_trailing_dir_sep_1(sb, 1);
+}
+
+void strbuf_trim_trailing_not_dir_sep(struct strbuf *sb)
+{
+ strbuf_trim_trailing_dir_sep_1(sb, 1);
+}
+
void strbuf_trim_trailing_newline(struct strbuf *sb)
{
if (sb->len > 0 && sb->buf[sb->len - 1] == '\n') {
diff --git a/strbuf.h b/strbuf.h
index f6dbb9681ee..b936f45ffad 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -189,6 +189,8 @@ void strbuf_ltrim(struct strbuf *sb);
/* Strip trailing directory separators */
void strbuf_trim_trailing_dir_sep(struct strbuf *sb);
+/* Strip trailing not-directory separators */
+void strbuf_trim_trailing_not_dir_sep(struct strbuf *sb);
/* Strip trailing LF or CR/LF */
void strbuf_trim_trailing_newline(struct strbuf *sb);
Then:
$ for str in a / b/ /c /d/ /e/ /f/g /h/i/ j/k l//m n/o/p //q/r/s/t; do STR=$str ./git version; done 2>&1 | grep :
EQ: |
EQ: / | /
EQ: b/ | b/
EQ: / | /
EQ: /d/ | /d/
EQ: /e/ | /e/
EQ: /f/ | /f/
EQ: /h/i/ | /h/i/
EQ: j/ | j/
EQ: l// | l//
EQ: n/o/ | n/o/
EQ: //q/r/s/ | //q/r/s/
I.e. for those inputs it's the same as the existing
strbuf_trim_trailing_dir_sep() with an inverted test. Is there some edge
case that I'm missing?
