On Mon, Oct 21, 2024 at 02:24:38PM +0000, Usman Akinyemi wrote:

On Mon, Oct 21, 2024 at 2:01 PM Patrick Steinhardt [off-list ref] wrote:

quoted

On Fri, Oct 18, 2024 at 01:52:59PM +0000, Usman Akinyemi via GitGitGadget wrote:

quoted

From: Usman Akinyemi <redacted>

Replaced atoi() with strtol_i() for parsing conflict-marker-size to
improve error handling. Invalid values, such as those containing letters
now trigger a clear error message.
Updated the test to verify invalid input handling.

When starting a new paragraph we typically have an empty line between
the paragraphs. We also tend to write commit messages as if instructing
the code to change. So instead of "Replaced atoi() with..." you'd say
"Replace atoi() with", and instead of "Updated the test...", you'd say
"Update the test ...".

The same applies to your other commits, as well.

Thanks for noting, Patrick.

quoted

These are a bit curious. As your test demonstrates, we retrieve the
values from the "gitattributes" file. And given that the file tends to be
checked into the repository, you can now basically break somebody elses
commands by having an invalid value in there.

That makes me think that we likely shouldn't die here. We may print a
warning, but other than that we should likely continue and use the
DEFAULT_CONFLICT_MARKER_SIZE.

Ohh, I understand. Philip suggested this. For the warning, will I just
use printf statement or what function to print the statement ?
Also, how do I test the print warning statement ?

You can use warning() instead of die(), which will also print the
message to stderr. You can redirect stderr to a separate file in your
test, and then grep or test_grep that to ensure that you see the warning
message.

These messages should also be marked for translation (with `_()`), so
the result will look something like:

    if (strtol_i(check->items[0].value, 10, &marker_size))
            warning(_("invalid marker-size '%s', expecting an integer"),
                    check->items[0].value);

Thanks,
Taylor