Re: [PATCH v3 0/5] notes.c: introduce "--no-blank-line" option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 22, 2022 at 4:30 AM Teng Long <dyroneteng@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> > Taking a step back, perhaps think of this in terms of "separator". The
> > default behavior is to insert "\n" as a separator between notes. If
> > you add a --separator option, then users could supply their own
> > separator, such as "----\n" or, in your case, "" to suppress the blank
> > line.
>
> There is another question for me, if the separator we passed contains "\n"
> string , the argument the cmd receives will need to tranfer to '\n' character
> instead to make sure it's a linebreak but not a "\n" instead.
>
> So maybe like:
> +static void insert_separator(struct strbuf *message, const char *separator)
> +{
> +               while (*separator) {
> +                       if (*separator == '\\'){
> +                               switch (separator[1]) {
> +                                       case 'n':
> +                                               strbuf_addstr(&transfered, "\n");
> +                                               separator++;
> +                                               break;
> + [...]
> +                       separator++;
> +               }
> +}
>
> If the above is understood correctly, is there an api that handles escape
> characters already in the existing code (I haven't found one so far, so I need
> to confirm and replace it if there is one). In addition, the insert_separator
> function above handles three special characters \t\n\r. Do we need more?

You could probably use unquote_c_style() from quote.[hc]; something like this:

    struct strbuf orig = STRBUF_INIT;
    struct strbuf unquoted = STRBUF_INIT;
    strbuf_addf(&orig, "\"%s\"", separator);
    if (unquote_c_style(&unquoted, orig.buf, NULL) < 0) {
        strbuf_release(&unquoted);
        strbuf_release(&orig);
        die(_("some suitable error message"));
    }
    /* unquote succeeded -- use "unquoted" here */

However, I suspect that this is overkill, and you should explore
simpler ideas first.

For instance, it is perfectly acceptable to embed newlines directly in
shell strings, so this would work just fine without having to write
any extra string-unquoting code:

    % git notes add --separator='---
    ' <object>

But, I think you can make this even friendlier without having to do
any extra coding to support string-unquoting. In particular, use this
heuristic:

    - if the separator is zero-length, use it as-is
    - otherwise, if the separator ends with a newline, use it as-is
    - otherwise add a newline to the separator

In other words:

    if (!separator)
        separator = "\n"; /* default is one blank line */
    if (*separator == '\0')
        /* separator is empty; use as-is (no blank line) */
    else if (separator[strlen(separator) - 1] == '\n')
        /* user supplied newline; use as-is */
    else
        /* separator lacks newline; add it ourselves */

With the above logic, this defaults to a blank line between notes:

    % git notes add ...

this has no blank line between notes:

    % git notes add --separator='' ...

this uses a "---" + "\n" as separator:

    % git notes add --separator='---' ...

as does this:

    % git notes add --separator='---
    ' ...

and this places two blank lines between notes:

    % git notes add --separator='

    ' ...



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux