I would like to work on this task.

This task has been assigned to Andromeda Galaxy. You have 100 hours to complete this task, good luck!

The work on this task is ready to be reviewed.

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Perhaps I missed the task/discussion where the need for this API addition came up?  I'm not sure I see the value and, more importantly, your patch doesn't actually reduce duplication from what I can tell.  On the contrary, there are two symbols getting added to public API and the overall line count increases.

The point is to substantially reduce our complexity and line count. ;)

If this was preceded by a discussion that I missed, please point me there or note the time if it was on IRC.

There was a big discussion on IRC with me, Stragus, and starseeker, last night around 16:00-17:30 PST where we talked a lot about how I noticed that there was a lot of duplication in the tests with regard to argument parsing: CMake won't pass empty strings like "" through to the tests, so currently each one has to implement its own parsing; I've found at least 5 tests, but most of them have already implemented some version of parsing for the empty string, with ~3 of them making the decision based on the number of argc (which only works when there is only one nullable argument).

I was originally planning on submitting this under fixing the last test that didn't have its own parsing for empty implemented, but during our discussion about the best way to do it, starseeker pushed a patch that fixed it in a different way, so when we eventually came to a consensus about what to do, I figured that this task was the most appropriate one, as future application of this function across tests should significantly reduce duplication.  If there was another option in the future that all the tests should have enabled, e.g. the ability to read test data from a file, this function could also possibly remove a large amount of duplication.

The work on this task is ready to be reviewed.

I would assert that this should still not become public LIBBU api.  It could live in the tests subdirectory as a helper function.  Better yet, it could probably exist as a macro, live in a header, and just be #included by the tests that want to use it instead of requiring compilation.

That said, it's not a very complicated function at all either.  I think at least some of the tests that can leverage this function (more than one) should be updated to use it as part of this task, if not all of them.

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

I did consider moving it into a tests subdirectory, but couldn't because tests from multiple libraries could use it.  That said, I don't mind implementing a few more tests using it; the bulk of the time that I spent on the task was spent in trying to a) first fix cmake (which proved impossible, even after queries on #cmake) and then b) come up with the best approach for removing the duplication... An api function was the best approach I could come up with, after talking over the disadvantages of some of the other approaches with starseeker and Stragus.... Would you mind continuing this discussion on IRC? I'll add a few more tests using it now...

The work on this task is ready to be reviewed.

The new version of the diff has several more tests using it, which I believe includes most of the users in libbu; unfortunately, locating which tests could benefit from this is sometimes difficult, especially since in some cases it appears that when a test was written this behavior wasn't noticed, and actually did the wrong test (see bu_dirname.c for an example of this; the _empty test actually acted identically to the _null test prior to this patch).

The -v3-alternate diff (second one; the first one was missing a file, sorry!) file includes the above extra tests but with TEST_ARGV() implemented in a macro in a new private header that tests that require the macro include.

I won't be able to get back into discussions on IRC for quite a while, but this looks a lot better to me.  I still don't think this belongs as public API (which everything in the top-level include/ directory is by definition), but I do understand that the logic is needed in all of the tests dirs.

That said, I was wondering if you tried passing in a nul-byte string or even a nul-byte string with trailing characters?

CMake might very well see it as a non-empty string (technically, not even "" is an 'empty' string .. it contains a single nul-byte.  Basically, it'd mean the CMake callers would pass something like "\0" or "\0whatever" instead of "" or (now) "#EMPTY".  Either is functionally equivalent as an empty string and any string function will terminate the same, however CMake may treat them better.  If that works, it'd obviate the need for adding boilerplate logic to all tests.

Thanks for the additional refactoring, and improvement all around and makes this more worthwhile.

Congratulations, this task has been completed successfully.