On 27 Jan 2021, Daniel Shahaf wrote:
You're welcome. I see only a few more nits; see below. (Some of
them I noticed in the first iteration too, but I didn't want to
pick too many nits then.) I'm happy that the patch is correct
and committable, though it won't make the "Major features in this
minor release" shortlist at the top of CHANGES.
Mmm, probably not, yeah :-).
Okay, I've addressed all your points -- thanks for the second
review! -- and the revised version is attached. If you don't spot
any more nits, I'll commit. (But more nits are certainly welcome,
of course.)
The message I'm replying to and your message dated "Tue, 26 Jan
2021 22:23:52 -0600" were threaded at the same level (sibling
nodes in my MUA's tree display), even though the latter quoted
the former. I haven't investigated.
That's probably due to my having hit 'Reply' on one message and
then manually copying-and-pasting some quoted text from another
message later in the same thread. (Sometimes I do that when it's
easier than re-starting my reply.)
Best regards, -Karl
[[[
Distinguish between regular properties and revprops in tmpfile name.
* subversion/svn/propedit-cmd.c
(svn_cl__propedit): In the revprop case, create a tmpfile name that
indicates that a revprop is being edited and on which revision.
* subversion/tests/cmdline/prop_tests.py
(tmpfile_name_matches_prop_type): New test function.
(test_list): Run it.
Review by: danielsh
]]]
Index: subversion/svn/propedit-cmd.c
===================================================================
--- subversion/svn/propedit-cmd.c (revision 1885938)
+++ subversion/svn/propedit-cmd.c (working copy)
@@ -143,7 +143,9 @@
SVN_ERR(svn_cmdline__edit_string_externally(
&propval, NULL,
opt_state->editor_cmd, temp_dir,
- propval, "svn-prop",
+ propval,
+ apr_psprintf(pool, "svn-revprop-r%ld",
+ opt_state->start_revision.value.number),
ctx->config,
svn_prop_needs_translation(pname),
opt_state->encoding, pool));
Index: subversion/tests/cmdline/prop_tests.py
===================================================================
--- subversion/tests/cmdline/prop_tests.py (revision 1885938)
+++ subversion/tests/cmdline/prop_tests.py (working copy)
@@ -2829,6 +2829,38 @@
expected_status,
extra_files=extra_files)
+
+# Test that editing a regular property creates a temporary file named
+# "svn-prop.tmp" whereas editing a revprop results in a temporary file
+# named "svn-revprop-rN.tmp" (where "N" is the number of the revision
+# whose revprop would be edited).
+def tmpfile_name_matches_prop_type(sbox):
+ "propedit tmpfile name matches property type"
+
+ sbox.build(read_only=True)
+
+ # We want the editor invocation to fail -- all we care about is the
+ # name of the tmpfile left over after that failure. I'm guessing
+ # you don't have a editor named this on your system:
+ non_editor = 'af968da2ce9'
+
+ svntest.actions.run_and_verify_svn(
+ None,
+ '.*' + re.escape(non_editor) + r'.*svn-revprop-r1\.tmp.*',
+ 'propedit', '--revprop',
+ '--editor-cmd', non_editor,
+ '-r1', 'svn:log',
+ sbox.repo_url)
+
+ svntest.actions.run_and_verify_svn(
+ None,
+ '.*' + re.escape(non_editor) + r'.*svn-prop\.tmp.*',
+ 'propedit',
+ '--editor-cmd', non_editor,
+ 'ignored-propname',
+ sbox.ospath('A/mu'))
+
+
########################################################################
# Run the tests
@@ -2880,6 +2912,7 @@
iprops_list_abspath,
wc_propop_on_url,
prop_conflict_root,
+ tmpfile_name_matches_prop_type,
]
if __name__ == '__main__':
