Author: hartmannathan Date: Fri May 31 13:40:56 2024 New Revision: 1918076 URL: http://svn.apache.org/viewvc?rev=1918076&view=rev Log: Factor-out parse of the --change argument from command line to libsvn_subr.
These changes factor-out the function, which parses a change revision, from the command line interface to the libsvn_subr library and add the unit tests on the function. * subversion/include/svn_opt.h (svn_opt_parse_revision_to_range): Declare new function. * subversion/libsvn_subr/opt.c (svn_opt_parse_revision_to_range): Factor-out the function from the svn.c:sub_main() function. * subversion/svn/svn.c (sub_main): Use the factored-out function instead of doing the work inline. (includes): Remove private/svn_opt_private.h, because it is no longer needed. * subversion/tests/cmdline/diff_tests.py (diff_invalid_change_arg): Update the test expectations to account for the updated error messages. * subversion/tests/libsvn_subr/opt-test.c (includes): Include private/svn_opt_private.h to use the svn_opt__revision_to_string() function. (revision_ranges_to_string): New function. (test_svn_opt_parse_change_to_range): New test. (test_funcs): Run new test. See the dev@ mail thread "Move change revision parser to libsvn_subr" started 21 May 2024, archived: https://lists.apache.org/thread/2ghkw77b858348y2zv8tsytljw3skfhv See also: r1917944 and r1917864 (fixes in the --change arg parser). Patch by: Timofey Zhakov (tima {at} chemodax _dot_ net) Review by: hartmannathan Modified: subversion/trunk/subversion/include/svn_opt.h subversion/trunk/subversion/libsvn_subr/opt.c subversion/trunk/subversion/svn/svn.c subversion/trunk/subversion/tests/cmdline/diff_tests.py subversion/trunk/subversion/tests/libsvn_subr/opt-test.c Modified: subversion/trunk/subversion/include/svn_opt.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_opt.h?rev=1918076&r1=1918075&r2=1918076&view=diff ============================================================================== --- subversion/trunk/subversion/include/svn_opt.h (original) +++ subversion/trunk/subversion/include/svn_opt.h Fri May 31 13:40:56 2024 @@ -534,6 +534,30 @@ svn_opt_parse_revision_to_range(apr_arra apr_pool_t *pool); /** + * Parse @a arg, where @a arg is "N", "-N", "N-M" into a + * @c svn_opt_revision_range_t and push that onto @a opt_ranges. + * + * - If @a arg is "N", set the @c start field of the + * @c svn_opt_revision_range_t to N-1 and @c end field to N. + * + * - If @a arg is "-N", set the @c start field of the + * @c svn_opt_revision_range_t to N and @c end field to N-1. + * + * - If @a arg is "N-M", set the @c start field of the + * @c svn_opt_revision_range_t to N-1 and @c end field to M. + * + * If @a arg is invalid, return -1; else return 0. + * + * Use @a result_pool to allocate @c svn_opt_revision_range_t pushed to the + * array. + * + * @since New in 1.15. + */ +int svn_opt_parse_change_to_range(apr_array_header_t *opt_ranges, + const char *arg, + apr_pool_t *result_pool); + +/** * Resolve peg revisions and operational revisions in the following way: * * - If @a is_url is set and @a peg_rev->kind is Modified: subversion/trunk/subversion/libsvn_subr/opt.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/opt.c?rev=1918076&r1=1918075&r2=1918076&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_subr/opt.c (original) +++ subversion/trunk/subversion/libsvn_subr/opt.c Fri May 31 13:40:56 2024 @@ -629,6 +629,86 @@ svn_opt_parse_revision_to_range(apr_arra return 0; } +int svn_opt_parse_change_to_range(apr_array_header_t *opt_ranges, + const char *arg, + apr_pool_t *result_pool) +{ + char *end; + svn_revnum_t changeno, changeno_end; + const char *s = arg; + svn_boolean_t is_negative; + + /* Check for a leading minus to allow "-c -r42". + * The is_negative flag is used to handle "-c -42" and "-c -r42". + * The "-c r-42" case is handled by strtol() returning a + * negative number. */ + is_negative = (*s == '-'); + if (is_negative) + s++; + + /* Allow any number of 'r's to prefix a revision number. */ + while (*s == 'r') + s++; + changeno = changeno_end = strtol(s, &end, 10); + if (end != s && *end == '-') + { + /* Negative number in range not supported with -c */ + if (changeno < 0 || is_negative) + return -1; + + s = end + 1; + while (*s == 'r') + s++; + changeno_end = strtol(s, &end, 10); + + /* Negative number in range not supported with -c */ + if (changeno_end < 0) + return -1; + } + + /* Non-numeric change argument given to -c? */ + if (end == arg || *end != '\0') + return -1; + + /* There is no change 0 */ + if (changeno == 0 || changeno_end == 0) + return -1; + + /* The revision number cannot contain a double minus */ + if (changeno < 0 && is_negative) + return -1; + + if (is_negative) + changeno = -changeno; + + /* Figure out the range: + -c N -> -r N-1:N + -c -N -> -r N:N-1 + -c M-N -> -r M-1:N for M < N + -c M-N -> -r M:N-1 for M > N + -c -M-N -> error (too confusing/no valid use case) + */ + if (changeno > 0) + { + if (changeno <= changeno_end) + changeno--; + else + changeno_end--; + } + else + { + changeno = -changeno; + changeno_end = changeno - 1; + } + + APR_ARRAY_PUSH(opt_ranges, + svn_opt_revision_range_t *) + = svn_opt__revision_range_from_revnums(changeno, changeno_end, + result_pool); + + return 0; +} + svn_error_t * svn_opt_resolve_revisions(svn_opt_revision_t *peg_rev, svn_opt_revision_t *op_rev, Modified: subversion/trunk/subversion/svn/svn.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/svn.c?rev=1918076&r1=1918075&r2=1918076&view=diff ============================================================================== --- subversion/trunk/subversion/svn/svn.c (original) +++ subversion/trunk/subversion/svn/svn.c Fri May 31 13:40:56 2024 @@ -55,7 +55,6 @@ #include "shelf2-cmd.h" #include "shelf-cmd.h" -#include "private/svn_opt_private.h" #include "private/svn_cmdline_private.h" #include "private/svn_subr_private.h" #include "private/svn_utf_private.h" @@ -2344,98 +2343,18 @@ sub_main(int *exit_code, int argc, const for (i = 0; i < change_revs->nelts; i++) { - char *end; - svn_revnum_t changeno, changeno_end; const char *change_str = APR_ARRAY_IDX(change_revs, i, const char *); - const char *s = change_str; - svn_boolean_t is_negative; - /* Check for a leading minus to allow "-c -r42". - * The is_negative flag is used to handle "-c -42" and "-c -r42". - * The "-c r-42" case is handled by strtol() returning a - * negative number. */ - is_negative = (*s == '-'); - if (is_negative) - s++; - - /* Allow any number of 'r's to prefix a revision number. */ - while (*s == 'r') - s++; - changeno = changeno_end = strtol(s, &end, 10); - if (end != s && *end == '-') - { - if (changeno < 0 || is_negative) - { - return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, - NULL, - _("Negative number in range (%s)" - " not supported with -c"), - change_str); - } - s = end + 1; - while (*s == 'r') - s++; - changeno_end = strtol(s, &end, 10); - - if (changeno_end < 0) - { - return svn_error_createf( - SVN_ERR_CL_ARG_PARSING_ERROR, NULL, - _("Negative number in range (%s)" - " not supported with -c"), - change_str); - } - } - if (end == change_str || *end != '\0') + if (svn_opt_parse_change_to_range(opt_state.revision_ranges, + change_str, pool) != 0) { return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL, - _("Non-numeric change argument (%s) " - "given to -c"), change_str); - } - - if (changeno == 0 || changeno_end == 0) - { - return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL, - _("There is no change 0")); - } - - /* The revision number cannot contain a double minus */ - if (changeno < 0 && is_negative) - { - return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL, - _("Non-numeric change argument " - "(%s) given to -c"), change_str); - } - - if (is_negative) - changeno = -changeno; - - /* Figure out the range: - -c N -> -r N-1:N - -c -N -> -r N:N-1 - -c M-N -> -r M-1:N for M < N - -c M-N -> -r M:N-1 for M > N - -c -M-N -> error (too confusing/no valid use case) - */ - if (changeno > 0) - { - if (changeno <= changeno_end) - changeno--; - else - changeno_end--; - } - else - { - changeno = -changeno; - changeno_end = changeno - 1; + _("Syntax error in change argument " + "'%s'"), change_str); } opt_state.used_change_arg = TRUE; - APR_ARRAY_PUSH(opt_state.revision_ranges, - svn_opt_revision_range_t *) - = svn_opt__revision_range_from_revnums(changeno, changeno_end, - pool); } } break; Modified: subversion/trunk/subversion/tests/cmdline/diff_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/diff_tests.py?rev=1918076&r1=1918075&r2=1918076&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/diff_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/diff_tests.py Fri May 31 13:40:56 2024 @@ -5336,33 +5336,33 @@ def diff_invalid_change_arg(sbox): svntest.actions.run_and_verify_svn( None, - (r'.*svn: E205000: Non-numeric change argument \(--1\) given to -c'), + (r'.*svn: E205000: Syntax error in change argument \'--1\''), 'diff', sbox.wc_dir, '-c', '--1') svntest.actions.run_and_verify_svn( None, - (r'.*svn: E205000: Non-numeric change argument \(-r-1\) given to -c'), + (r'.*svn: E205000: Syntax error in change argument \'-r-1\''), 'diff', sbox.wc_dir, '-c', '-r-1') svntest.actions.run_and_verify_svn( None, - (r'.*svn: E205000: Negative number in range \(1--3\) not supported with -c'), + (r'.*svn: E205000: Syntax error in change argument \'1--3\''), 'diff', sbox.wc_dir, '-c', '1--3') # 'r' is not a number svntest.actions.run_and_verify_svn( None, - (r'.*svn: E205000: Non-numeric change argument \(r1--r3\) given to -c'), + (r'.*svn: E205000: Syntax error in change argument \'r1--r3\''), 'diff', sbox.wc_dir, '-c', 'r1--r3') svntest.actions.run_and_verify_svn( None, - (r'.*svn: E205000: Negative number in range \(r1-r-3\) not supported with -c'), + (r'.*svn: E205000: Syntax error in change argument \'r1-r-3\''), 'diff', sbox.wc_dir, '-c', 'r1-r-3') svntest.actions.run_and_verify_svn( None, - (r'.*svn: E205000: There is no change 0'), + (r'.*svn: E205000: Syntax error in change argument \'1-0\''), 'diff', sbox.wc_dir, '-c', '1-0') ######################################################################## Modified: subversion/trunk/subversion/tests/libsvn_subr/opt-test.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/opt-test.c?rev=1918076&r1=1918075&r2=1918076&view=diff ============================================================================== --- subversion/trunk/subversion/tests/libsvn_subr/opt-test.c (original) +++ subversion/trunk/subversion/tests/libsvn_subr/opt-test.c Fri May 31 13:40:56 2024 @@ -27,6 +27,7 @@ #include "../svn_test.h" #include "svn_opt.h" +#include "private/svn_opt_private.h" static svn_error_t * @@ -190,6 +191,78 @@ test_svn_opt_args_to_target_array2(apr_p return SVN_NO_ERROR; } +static const char * +revision_ranges_to_string(apr_array_header_t *ranges, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + svn_stringbuf_t *result = svn_stringbuf_create_empty(result_pool); + int i; + + for (i = 0; i < ranges->nelts; i++) + { + svn_opt_revision_range_t *range = + APR_ARRAY_IDX(ranges, i, svn_opt_revision_range_t *); + + if (i > 0) + { + svn_stringbuf_appendcstr(result, ","); + } + + svn_stringbuf_appendcstr(result, + svn_opt__revision_to_string(&range->start, + scratch_pool)); + svn_stringbuf_appendcstr(result, ":"); + + svn_stringbuf_appendcstr(result, + svn_opt__revision_to_string(&range->end, + scratch_pool)); + } + + return result->data; +} + +static svn_error_t* +test_svn_opt_parse_change_to_range(apr_pool_t *pool) +{ + int i; + static struct { + const char *input; + const int expected_rv; + const char *expected_range; + } const tests[] = { + { "123", 0, "122:123"}, + { "r123", 0, "122:123"}, + { "-123", 0, "123:122"}, + { "-r123", 0, "123:122"}, + { "r-123", 0, "123:122"}, + { "--123", -1, ""}, + { "-r-123", -1, ""}, + { "123-456", 0, "122:456"}, + { "r123-r456", 0, "122:456"}, + { "--r123", -1, ""}, + { "123--456", -1, ""}, + { "1-0", -1, ""}, + { "0-1", -1, ""}, + }; + + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) + { + apr_array_header_t *ranges = + apr_array_make(pool, 0, sizeof(svn_opt_revision_range_t *)); + + SVN_TEST_INT_ASSERT( + svn_opt_parse_change_to_range(ranges, tests[i].input, pool), + tests[i].expected_rv); + + SVN_TEST_STRING_ASSERT( + revision_ranges_to_string(ranges, pool, pool), + tests[i].expected_range); + } + + return SVN_NO_ERROR; +} + /* The test table. */ @@ -202,6 +275,8 @@ static struct svn_test_descriptor_t test "test svn_opt_parse_path"), SVN_TEST_PASS2(test_svn_opt_args_to_target_array2, "test svn_opt_args_to_target_array2"), + SVN_TEST_PASS2(test_svn_opt_parse_change_to_range, + "test svn_opt_parse_change_to_range"), SVN_TEST_NULL };
