You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Grosberg <ma...@nolab.conman.org> on 2003/05/13 06:17:43 UTC
[PATCH][RESEND] Issue #1295 (take 5)
Anybody have a chance to look over my latest patch? Haven't heard anything
back for several days.
L8r,
Mark G.
========================================================================
Fix for issue #1295.
* subversion/libsvn_client/commit_util.c
(svn_client__condense_commit_items): Skip targets that have the
new SVN_CLIENT_COMMIT_ITEM_UNMARKED flag set.
(svn_client__sort_commit_item_urls): Always sort unmarked items
at the end of the list. This way, they are easy to remove.
* subversion/clients/cmdline/util.c
(get_item_path): [New] Factored out log path name generator.
(flag_unmarked): [New] Set the SVN_CLIENT_COMMIT_ITEM_UNMARKED flag
(log_msg_fatal_error): [New] Helper error factory function.
for paths not found in the specified hash.
(process_comitted_list): Parse the edited message to see what file
names remain and add them to a hash.
(svn_cl__get_log_message): Mark the entries in the comitted items
list that should not be comitted.
* subversion/include/svn_client.h
Add new flag: SVN_CLIENT_COMMIT_ITEM_UNMARKED
* subversion/include/svn_error_codes.h
Add new client error: SVN_ERR_CLIENT_NO_WORK
Add new command line error: SVN_ERR_CL_NO_LOG_MESSAGE_FILE_LIST
-------------------------------------------------------------------------
Index: include/svn_error_codes.h
===================================================================
--- include/svn_error_codes.h (revision 5857)
+++ include/svn_error_codes.h (working copy)
@@ -688,6 +688,11 @@
SVN_ERR_CLIENT_CATEGORY_START + 8,
"Revision range is not allowed")
+ SVN_ERRDEF (SVN_ERR_CLIENT_NO_WORK,
+ SVN_ERR_CLIENT_CATEGORY_START + 9,
+ "No work to do")
+
+
/* misc errors */
SVN_ERRDEF (SVN_ERR_BASE,
@@ -800,6 +805,10 @@
SVN_ERR_CL_CATEGORY_START + 8,
"Something is wrong with the log message's contents.")
+ SVN_ERRDEF (SVN_ERR_CL_NO_LOG_MESSAGE_FILE_LIST,
+ SVN_ERR_CL_CATEGORY_START + 9,
+ "No generated file list found")
+
SVN_ERROR_END
Index: include/svn_client.h
===================================================================
--- include/svn_client.h (revision 5857)
+++ include/svn_client.h (working copy)
@@ -292,6 +292,7 @@
#define SVN_CLIENT_COMMIT_ITEM_TEXT_MODS 0x04
#define SVN_CLIENT_COMMIT_ITEM_PROP_MODS 0x08
#define SVN_CLIENT_COMMIT_ITEM_IS_COPY 0x10
+#define SVN_CLIENT_COMMIT_ITEM_UNMARKED 0x20
/** @} */
/** The commit candidate structure. */
Index: libsvn_client/commit_util.c
===================================================================
--- libsvn_client/commit_util.c (revision 5857)
+++ libsvn_client/commit_util.c (working copy)
@@ -730,7 +730,13 @@
= *((const svn_client_commit_item_t * const *) a);
const svn_client_commit_item_t *item2
= *((const svn_client_commit_item_t * const *) b);
- return svn_path_compare_paths (item1->url, item2->url);
+
+ int cmp = ((item1->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED)
+ - (item2->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED));
+ if (cmp == 0)
+ cmp = svn_path_compare_paths (item1->url, item2->url);
+
+ return cmp;
}
@@ -751,6 +757,20 @@
qsort (ci->elts, ci->nelts,
ci->elt_size, svn_client__sort_commit_item_urls);
+ /* Delete unrequested items. */
+ for (i = ci->nelts; i > 0; i--)
+ {
+ item = (((svn_client_commit_item_t **) ci->elts)[i - 1]);
+ if (!(item->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED))
+ break;
+ }
+
+ ci->nelts = i;
+ if (ci->nelts == 0)
+ return svn_error_create (SVN_ERR_CLIENT_NO_WORK, NULL,
+ "No entires submitted for commit.");
+
+
/* Loop through the URLs, finding the longest usable ancestor common
to all of them, and making sure there are no duplicate URLs. */
for (i = 0; i < ci->nelts; i++)
Index: clients/cmdline/util.c
===================================================================
--- clients/cmdline/util.c (revision 5857)
+++ clients/cmdline/util.c (working copy)
@@ -414,8 +414,165 @@
}
-#define EDITOR_EOF_PREFIX "--This line, and those below, will be ignored--"
+#define EDITOR_EOF_PREFIX "--This line, and those below, " \
+ "are the list of files to commit--" \
+ APR_EOL_STR \
+ "--and will not be included in the log message--"
+/* Get the full path string for the specificed item. */
+static const char *
+get_item_path (svn_client_commit_item_t *item,
+ struct log_msg_baton *lmb,
+ apr_pool_t *pool)
+{
+ const char *item_path = item->path;
+
+ if (! item_path)
+ item_path = item->url;
+ else if (! *item_path)
+ item_path = ".";
+
+ if (item_path && lmb->base_dir)
+ item_path = svn_path_is_child (lmb->base_dir, item_path, pool);
+
+ /* If still no path, then just use current directory. */
+ if (! item_path)
+ item_path = ".";
+
+ return item_path;
+}
+
+/* Helper function to create fatal parse error messages. */
+static svn_error_t *
+log_msg_fatal_error (void)
+{
+ return svn_error_create (SVN_ERR_CL_BAD_LOG_MESSAGE, NULL,
+ "Aborting commit -- invalid log");
+}
+
+/* Parse the edited comitted message and store the named paths
+ in the hash. Failures of SVN_ERR_CL_NO_LOG_MESSAGE_FILE_LIST are
+ not fatal but cause all files to be comitted. */
+static svn_error_t *
+process_comitted_list (apr_hash_t *seen,
+ char *committed_list,
+ apr_pool_t *pool)
+{
+ char *end_str, *prev_str = NULL;
+
+ /* Find the generated message. */
+ end_str = committed_list;
+ while (end_str != NULL)
+ {
+ end_str = strstr (end_str, EDITOR_EOF_PREFIX);
+ if (end_str == NULL)
+ break;
+
+ end_str = end_str + (sizeof(EDITOR_EOF_PREFIX) - 1);
+ prev_str = end_str;
+ }
+
+ if (prev_str == NULL)
+ return svn_error_create (SVN_ERR_CL_NO_LOG_MESSAGE_FILE_LIST, NULL,
+ "Selective commit aborted; comitting all files.");
+
+ while (*prev_str != '\0')
+ {
+ if (!apr_isspace (*prev_str))
+ break;
+ prev_str++;
+ }
+
+ /* Now, parse each line and make sure that the committed item is named. */
+ for (;;)
+ {
+ char *end_of_path;
+
+ /* Skip leading whitespace. */
+ for (;;)
+ {
+ if (*prev_str == '\0')
+ return SVN_NO_ERROR;
+
+ if (!apr_isspace (*prev_str))
+ break;
+ prev_str++;
+ }
+ if (*prev_str == '\0')
+ break;
+
+ /* The next byte had better be one of the text mod's below. */
+ switch (*prev_str++)
+ {
+ case '_':
+ case 'R':
+ case 'A':
+ case 'D':
+ case 'M': break;
+ default: return log_msg_fatal_error ();
+ }
+
+ /* The next byte had better be one of the prop mod's below. */
+ switch (*prev_str++)
+ {
+ case ' ':
+ case 'M': break;
+ default: return log_msg_fatal_error ();
+ }
+
+ /* Skip more whitespace. */
+ for (;;)
+ {
+ if (*prev_str == '\0')
+ return log_msg_fatal_error ();
+ if (!apr_isspace(*prev_str))
+ break;
+ prev_str++;
+ }
+
+ /* Chop up the path. */
+ for (end_of_path = prev_str; *end_of_path != '\0'; end_of_path++)
+ {
+ if (*end_of_path == '\n')
+ break;
+ }
+ if (*end_of_path == '\0')
+ return log_msg_fatal_error ();
+
+ /* Truncate the string. The path is now in prev_str. */
+ *end_of_path++ = '\0';
+
+ /* Note the existence. */
+ apr_hash_set(seen, prev_str, APR_HASH_KEY_STRING, "");
+
+ /* Advance the pointer. */
+ prev_str = end_of_path;
+ }
+
+ return SVN_NO_ERROR;
+}
+
+/* Find the paths that are marked in the seen hash and set their
+ unmarked flags. */
+static void
+flag_unmarked (struct log_msg_baton *lmb,
+ apr_array_header_t *commit_items,
+ apr_hash_t *seen,
+ apr_pool_t *pool)
+{
+ int i;
+
+ for (i = 0; i < commit_items->nelts; i++)
+ {
+ svn_client_commit_item_t *item
+ = ((svn_client_commit_item_t **) commit_items->elts)[i];
+ const char *path = get_item_path (item, lmb, pool);
+
+ if (! apr_hash_get (seen, path, APR_HASH_KEY_STRING))
+ item->state_flags |= SVN_CLIENT_COMMIT_ITEM_UNMARKED;
+ }
+}
+
/* This function is of type svn_client_get_commit_log_t. */
svn_error_t *
svn_cl__get_log_message (const char **log_msg,
@@ -469,21 +626,9 @@
{
svn_client_commit_item_t *item
= ((svn_client_commit_item_t **) commit_items->elts)[i];
- const char *path = item->path;
+ const char *path = get_item_path (item, lmb, pool);
char text_mod = '_', prop_mod = ' ';
- if (! path)
- path = item->url;
- else if (! *path)
- path = ".";
-
- if (path && lmb->base_dir)
- path = svn_path_is_child (lmb->base_dir, path, pool);
-
- /* If still no path, then just use current directory. */
- if (! path)
- path = ".";
-
if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)
&& (item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD))
text_mod = 'R';
@@ -542,8 +687,20 @@
/* Strip the prefix from the buffer. */
if (message)
+ {
+ svn_error_t *list_err;
+ char *committed_list = apr_pstrdup (pool, msg2);
+ apr_hash_t *seen = apr_hash_make(pool);
+
+ list_err = process_comitted_list (seen, committed_list, pool);
+ if (list_err == SVN_NO_ERROR)
+ flag_unmarked (lmb, commit_items, seen, pool);
+ else if (list_err->apr_err != SVN_ERR_CL_NO_LOG_MESSAGE_FILE_LIST)
+ return (list_err);
+
truncate_buffer_at_prefix (&message->len, message->data,
EDITOR_EOF_PREFIX);
+ }
if (message)
{
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Robert Pluim <rp...@bigfoot.com>.
Garrett Rooney writes:
>
> On Tuesday, May 13, 2003, at 11:59 PM, Mark Grosberg wrote:
> > It is an "alternative" interface to something that could be done on the
> > command line. But it is a convenient one to some, none the less. On the
> > bright side, it is relatively transparent. It's nice to know that after
> > you type in a long log message the one file you added a "printf" in for
> > debugging can be quickly removed as you do your final checks for your
> > commit.
> >
I see where you're coming from, and I (sort of) agree. OTOH, I bet
I'm not the only one that's ever committed a subset of the files I'd
changed, confident that everything was OK, only to be LARTed by the
build manager because I broke the build. As a result, I've become a
'commit everything I changed or commit nothing' kind of person. So,
I'm -0 on this one.
> > How would Garrett feel about keeping this feature with a config option?
>
-1 on the config stuff. I think it should be there or not be there,
no half-way houses.
Robert
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Stefan Monnier <mo...@rum.cs.yale.edu>.
> Obviously you guys have more experience in dealing with version control
> systems as well as with adding new features to Subversion. I know just one
> thing for sure: I'm a die hard CVS fan (even though I don't know all its
> features) who has to use Perforce because of how SourceMage project is
> hosted, and editing a file list before committing is one of the features
> I actually appreciate. I think it's logical because it allows productive use
> of something that is static in CVS: list of files that have changed.
I think this boils down to whether `svn' should offer a nice UI
or whether it should concentrate on providing clean semantics.
For example, in the case of CVS, I also like to see the list of files before
I commit them, but I don't rely on the `cvs' command-line client for that,
but on the PCL-CVS front-end instead.
Also, when it comes to partial commits, the file granularity is often
not sufficient in my experience, so I need something more sophisticated
than what you suggest.
OTOH, I don't care at all how `svn commit' behaves when it runs $EDITOR
because I never use it in this way (I always write the message somewhere
else first and pass it as an argument). I tend to see such "niceties" as
either baggage or annoyances (when a batch process ends up trying to
execute $EDITOR because of some mistake somewhere), but if it can be
convenient for someone...
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by "Sergey A. Lipnevich" <se...@pisem.net>.
Sander Striker wrote:
>You mean the output produced by:
>
>$ svn status
>
>
>
Well, yes and no: I think `svn status' may be avoided before committing.
My thinking is that the output of svn status (in a commit editor) now
becomes editable, which I believe is good because I personally depended
on such feature of Perforce several times, in situations when many files
have changed in multiple directories.
Thank you!
Sergey.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Sander Striker <st...@apache.org>.
> From: Sergey A. Lipnevich [mailto:sergeyli@pisem.net]
> Sent: Wednesday, May 14, 2003 6:30 PM
> The difference I'm talking about is the one (e.g. in a restaurant)
> between knowing what's on the menu and having the menu in front of you.
> If you're a fequent guest or you always know what you want, you won't
> need a menu. If you want to refresh your memory or want to look up
> what's there to make sure you didn't miss anything. I'm assuming you
> know what's inside each dish (file) once you see the name.
You mean the output produced by:
$ svn status
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by "Sergey A. Lipnevich" <se...@pisem.net>.
cmpilato@collab.net wrote:
>"Sergey A. Lipnevich" <se...@pisem.net> writes:
>
>
>
>>Consider a documentation directory with 150 xml files all named
>>similarly: chaper{001-150}.xml. If you've made changes in twenty five
>>of them to replace "hyper" with "super", and in another three to
>>describe a new feature, you'll have to first lookup what has changed
>>(step 1), and then commit separately (steps 2-3). Having such a
>>feature removes step 1: bring up the commit editor, sort out
>>unnecessary files, commit.
>>
>>
>
>See, this is the part of your argument that doesn't make sense. How
>does this coder "sort out the unnecessary files"? By "looking up what
>has changed" in those files. Don't you see, he *is* doing step 1 in
>both scenarios, just at different times. The only way he can truly
>skip step 1 is if he already knows which paths were part of which
>group of changes, in which case this reverts to what I stated as the
>only benefit of this system (see below, or my previous message).
>
The difference I'm talking about is the one (e.g. in a restaurant)
between knowing what's on the menu and having the menu in front of you.
If you're a fequent guest or you always know what you want, you won't
need a menu. If you want to refresh your memory or want to look up
what's there to make sure you didn't miss anything. I'm assuming you
know what's inside each dish (file) once you see the name.
>Right. Which is *exactly* the one benefit I mentioned:
>
> "The only real benefit is that we've saved the guy from having to
> type in a bunch of paths at the command-line."
>
>
No, the benefit of collecting all changes for coder's observation before
committing *and* allowing this coder to alter it.
>And likewise, please do not understate the steps in your scenarios so
>that they suit your particular argument.
>
>
Obviously you guys have more experience in dealing with version control
systems as well as with adding new features to Subversion. I know just
one thing for sure: I'm a die hard CVS fan (even though I don't know all
its features) who has to use Perforce because of how SourceMage project
is hosted, and editing a file list before committing is one of the
fetaures I actually appreciate. I think it's logical because it allows
productive use of something that is static in CVS: list of files that
have changed.
Thank you!
Sergey.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by cm...@collab.net.
"Sergey A. Lipnevich" <se...@pisem.net> writes:
> Consider a documentation directory with 150 xml files all named
> similarly: chaper{001-150}.xml. If you've made changes in twenty five
> of them to replace "hyper" with "super", and in another three to
> describe a new feature, you'll have to first lookup what has changed
> (step 1), and then commit separately (steps 2-3). Having such a
> feature removes step 1: bring up the commit editor, sort out
> unnecessary files, commit.
See, this is the part of your argument that doesn't make sense. How
does this coder "sort out the unnecessary files"? By "looking up what
has changed" in those files. Don't you see, he *is* doing step 1 in
both scenarios, just at different times. The only way he can truly
skip step 1 is if he already knows which paths were part of which
group of changes, in which case this reverts to what I stated as the
only benefit of this system (see below, or my previous message).
> Another scenario is when both changes are scattered among several
> directories: "hyper" is changed for "super" in dir1/foo and
> dir2/bar, a new feature implemented in dir1/extrafoo and
> dir2/minibar. Instead of typing, one would pick the suitable lines
> and plunge ahead.
Right. Which is *exactly* the one benefit I mentioned:
"The only real benefit is that we've saved the guy from having to
type in a bunch of paths at the command-line."
> As I said, please do not overestimate coder discipline in other
> projects.
And likewise, please do not understate the steps in your scenarios so
that they suit your particular argument.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by "Sergey A. Lipnevich" <se...@pisem.net>.
cmpilato@collab.net wrote:
>Sergey <se...@pisem.net> writes:
>
>
>
>>This feature also enables the scenario where a coder would be able
>>to easily collect files from different directories that have changed
>>in regards to feature 1, and remove files that changed in regards to
>>feature 2, which must go in a separate commit. Without such feature
>>a coder would rely on memory to pick files from those that have
>>changed, which is statistically by far less reliable than that of a
>>PC.
>>
>>
>
>This argument makes no sense.
>
Thank you :-)
> In neither scenario is "a PC" making
>the decision about what to commit. It's that same coder, simply
>putting off a decision he was going to make at the command-line until
>his editor pops up, and then making that decision. And how is he
>going to decide what stays in the commit? Well, he'll examine the
>paths in the commit message -- you know, the exact same way he'd have
>made the decision at the command-line using the output from 'svn
>status'.
>
The only difference being the list of files that have changed in front
of him/her, not in the head (waiting to be overlooked).
>
>The only real benefit is that we've saved the guy from having to type
>in a bunch of paths at the command-line. Now we can all argue the
>value of that benefit, but let's not make it sound like this feature
>is the saving grace from a world tainted by icky humans and their
>faulty circuits.
>
>
Consider a documentation directory with 150 xml files all named
similarly: chaper{001-150}.xml. If you've made changes in twenty five of
them to replace "hyper" with "super", and in another three to describe a
new feature, you'll have to first lookup what has changed (step 1), and
then commit separately (steps 2-3). Having such a feature removes step
1: bring up the commit editor, sort out unnecessary files, commit.
Repeat for second commit. Another scenario is when both changes are
scattered among several directories: "hyper" is changed for "super" in
dir1/foo and dir2/bar, a new feature implemented in dir1/extrafoo and
dir2/minibar. Instead of typing, one would pick the suitable lines and
plunge ahead.
As I said, please do not overestimate coder discipline in other projects.
Sergey.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by cm...@collab.net.
Sergey <se...@pisem.net> writes:
> This feature also enables the scenario where a coder would be able
> to easily collect files from different directories that have changed
> in regards to feature 1, and remove files that changed in regards to
> feature 2, which must go in a separate commit. Without such feature
> a coder would rely on memory to pick files from those that have
> changed, which is statistically by far less reliable than that of a
> PC.
This argument makes no sense. In neither scenario is "a PC" making
the decision about what to commit. It's that same coder, simply
putting off a decision he was going to make at the command-line until
his editor pops up, and then making that decision. And how is he
going to decide what stays in the commit? Well, he'll examine the
paths in the commit message -- you know, the exact same way he'd have
made the decision at the command-line using the output from 'svn
status'.
The only real benefit is that we've saved the guy from having to type
in a bunch of paths at the command-line. Now we can all argue the
value of that benefit, but let's not make it sound like this feature
is the saving grace from a world tainted by icky humans and their
faulty circuits.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Sergey <se...@pisem.net>.
Garrett Rooney wrote:
> Honestly, I use the feature in perforce, but primarily because I can't
> figure out how to tell perforce to do a 'p4 submit foo/bar.c
> blah/baz.c'. You can do wildcards, but that only works if you want
> everything in a directory, or you can do 'everything' and edit the list,
> at least as far as I can tell. Subversion's current way of dealing with
> it seems more intuitive, the perforce way seems like a bit of a hack.
I find it a very logical feature, one that allow a coder to actually control what's being committed at a moment where this
control is needed most (let's face it, not many projects follow the tight discipline of Subversion team). Contrary to what Karl
thinks, I don't see why it would cause any damage -- losing log message sounds like more damage to me. This feature also enables
the scenario where a coder would be able to easily collect files from different directories that have changed in regards to
feature 1, and remove files that changed in regards to feature 2, which must go in a separate commit. Without such feature a
coder would rely on memory to pick files from those that have changed, which is statistically by far less reliable than that of
a PC.
BTW, in response to Garrett's question, in Perforce one can create a new [numbered] changeset and then add bunches of files to
it to form desired set of files before `p4 submit'-ting. It's not convenient though, because one needs to remember this
changeset number and type it for every "add to changeset" and for "submit" operation.
So, I'm begging to accept this feature :-). Please?
Sergey.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Colin Watson <cj...@flatline.org.uk>.
On Wed, May 14, 2003 at 07:46:53AM -0400, Garrett Rooney wrote:
> Honestly, I use the feature in perforce, but primarily because I can't
> figure out how to tell perforce to do a 'p4 submit foo/bar.c
> blah/baz.c'. You can do wildcards, but that only works if you want
> everything in a directory, or you can do 'everything' and edit the
> list, at least as far as I can tell. Subversion's current way of
> dealing with it seems more intuitive, the perforce way seems like a
> bit of a hack.
Well, I kind of see your point, but when you display a list of files
it's not unreasonable for a user to expect to be able to edit them and
modify the commit accordingly. I'm quite prepared to believe it's
complex from an implementation point of view, and I do hate the way you
can't submit multiple files on the command line in Perforce, but after
spending a couple of years working in a Perforce-using shop I grew to
truly appreciate the file list editing feature when I had lots of files
open spread out all over my working copy. cd to the top level, 'p4
submit', delete the couple of files I didn't want; much faster than
trying to tab-complete a couple of dozen filenames or copy and paste
them from 'svn status' output.
There's another benefit to the Perforce approach for Subversion; it'll
be an easy way around the problem of trying to commit a propchange to a
directory when you've also changed some files in that directory. Instead
of 'svn ci --depth 0' and suchlike (which with all due respect I think
is a neat but horribly unintuitive idea - I wouldn't swear offhand to it
being '--depth 0' rather than '--depth 1', for instance, which isn't a
good sign), you could just 'svn ci' and delete all the file
modifications from the list, leaving only the propchange. Dead easy, and
you could teach it to a user in ten seconds just by demonstration.
Cheers,
--
Colin Watson [cjwatson@flatline.org.uk]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Tuesday, May 13, 2003, at 11:59 PM, Mark Grosberg wrote:
>
>
> On Tue, 13 May 2003, mark benedetto king wrote:
>
>> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=9845
>
> I took a look at this. It seems that Perforce does it, but has a crappy
> parser. I think my parser is quite intuitive. If you remove the marker
> line, the parsing is not done. If you alter the format of the part it
> is
> parsing, it errors out.
>
> It is an "alternative" interface to something that could be done on the
> command line. But it is a convenient one to some, none the less. On the
> bright side, it is relatively transparent. It's nice to know that after
> you type in a long log message the one file you added a "printf" in for
> debugging can be quickly removed as you do your final checks for your
> commit.
>
> How would Garrett feel about keeping this feature with a config option?
I'm ambivalent about it. Basically, if it's there, I'd rather not hide
it behind a config file, and i'd want to be sure that it's obvious from
looking at the template we load up that you can edit the list of files
(the 'everything below this line will be ignored' thing doesn't cut it,
since we're not ignoring it anymore). that said, I haven't looked at
the latest iteration of the patch, so perhaps you've dealt with that
already.
Honestly, I use the feature in perforce, but primarily because I can't
figure out how to tell perforce to do a 'p4 submit foo/bar.c
blah/baz.c'. You can do wildcards, but that only works if you want
everything in a directory, or you can do 'everything' and edit the
list, at least as far as I can tell. Subversion's current way of
dealing with it seems more intuitive, the perforce way seems like a bit
of a hack.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Mark Grosberg <ma...@nolab.conman.org>.
On Tue, 13 May 2003, mark benedetto king wrote:
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=9845
I took a look at this. It seems that Perforce does it, but has a crappy
parser. I think my parser is quite intuitive. If you remove the marker
line, the parsing is not done. If you alter the format of the part it is
parsing, it errors out.
It is an "alternative" interface to something that could be done on the
command line. But it is a convenient one to some, none the less. On the
bright side, it is relatively transparent. It's nice to know that after
you type in a long log message the one file you added a "printf" in for
debugging can be quickly removed as you do your final checks for your
commit.
How would Garrett feel about keeping this feature with a config option?
L8r,
Mark G.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by mark benedetto king <mb...@boredom.org>.
On Tue, May 13, 2003 at 09:46:49AM -0500, kfogel@collab.net wrote:
> Regarding issue #1295 specifically, I just noticed now that the issue
> is marked "Resolved->Invalid" by Mark Benedetto King, saying:
>
> > This was discussed earlier on the dev mailing list. The decision
> > was that "svn st > foo; $EDITOR foo; svn commit --targets foo"
> > was just as good, and less likely to cause problems for CVS
> > users.
> >
> > I'm marking the issue invalid; if for some reason the command
> > sequence above doesn't meet your requirements, please re-open it.
>
> I don't have any memory of this consensus; maybe MBK can say more?
>
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=9845
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Mark Grosberg <ma...@nolab.conman.org>.
On 13 May 2003 kfogel@collab.net wrote:
> After a few cycles of this, there is nothing obviously wrong with the
> patch. It just needs a detailed review & test, followed (we hope) by
> application. That's all much more time-consuming than the earlier
Is there anything I can do to help? I've tested it with my own SVN repos
since I posted it, no problems found so far. Problems in it would probably
be found pretty quick. The code is in the path for every commit with the
command line client?
> This pattern is exacerbated when the patch is for a feature that
> doesn't block 1.0. It's much lower on everyone's priority list then
True. But it's a usability thing and really effects UI. It would be great
to get it out there for 1.0 so people get familiar with that kind of
behavior.
> Regarding issue #1295 specifically, I just noticed now that the issue
> is marked "Resolved->Invalid" by Mark Benedetto King, saying:
Actually, I did the first patch before the issue got marked Invalid. :-)
L8r,
Mark G.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by kf...@collab.net.
Mark Grosberg <ma...@nolab.conman.org> writes:
> Anybody have a chance to look over my latest patch? Haven't heard anything
> back for several days.
Sorry for the delay Mark -- I think everyone's just been busy.
There's an unfortunate pattern here, one familiar to longtime list
watchers :-). A patch comes in, but has some obvious and easily
correctable problems, such as incomplete doc strings, wrong naming
conventions, log message, whatever. So it gets feedback right away,
because the feedback is so easy to give.
After a few cycles of this, there is nothing obviously wrong with the
patch. It just needs a detailed review & test, followed (we hope) by
application. That's all much more time-consuming than the earlier
feedback, so it's a while before someone gets time to do it.
This pattern is exacerbated when the patch is for a feature that
doesn't block 1.0. It's much lower on everyone's priority list then
(which is not to disprize the effort you obviously put into it, but
the review/apply part of the cycle is about someone else's time...)
Regarding issue #1295 specifically, I just noticed now that the issue
is marked "Resolved->Invalid" by Mark Benedetto King, saying:
> This was discussed earlier on the dev mailing list. The decision
> was that "svn st > foo; $EDITOR foo; svn commit --targets foo"
> was just as good, and less likely to cause problems for CVS
> users.
>
> I'm marking the issue invalid; if for some reason the command
> sequence above doesn't meet your requirements, please re-open it.
I don't have any memory of this consensus; maybe MBK can say more?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> >Anybody have a chance to look over my latest patch? Haven't heard anything
> >back for several days.
>
> I remember one relevant comment that you haven't addressed yet (see
> attached message).
Well, let's not encourage poor Mark to keep putting time into this
unless it's really going to get merged in. Right now, we don't have a
consensus that the feature is even desirable, let alone desirable for
1.0.
I hate to say it, but the more I think about it, the less I'd like to
see it merged in. The possibility for unexpected consequences seems
awfully high. I don't mean bugs in the code, necessarily, just
unexpectednesses in the user's experience.
I certainly wouldn't veto it, if people feel strongly that it should
go in. But let's please settle *that* question before continuing the
patch review cycle, which could waste Mark's time (and annoy him,
though he's been awfully forgiving so far).
-K
> Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
> From: cmpilato@collab.net
> Subject: Re: [PATCH] Issue #1295 (take 2, with log)
> To: Mark Grosberg <ma...@nolab.conman.org>
> Cc: Branko Čibej <br...@xbc.nu>, dev@subversion.tigris.org
> Date: 09 May 2003 02:58:12 -0500
> Reply-To: cmpilato@collab.net
>
> Mark Grosberg <ma...@nolab.conman.org> writes:
>
> > I'd really rather not touch Python. I know Perl, but not Python and this
> > would take me a considerable amount of time to learn.
> >
> > > Modifying the commit list in a message passed in with -F should work, right?
> >
> > I don't know. The code is pretty hard to follow. Not even sure how -F
> > works.
>
> Your code should already do this correctly. However, I have another
> suggestion:
>
> > @@ -542,8 +671,16 @@
> >
> > /* Strip the prefix from the buffer. */
> > if (message)
> > + {
> > + char *commited_list = apr_pstrdup (pool, msg2);
> > + apr_hash_t *seen = apr_hash_make(pool);
> > +
> > + process_comitted_list (seen, commited_list, pool);
> > + flag_unmarked (lmb, commit_items, seen, pool);
> > +
> > truncate_buffer_at_prefix (&message->len, message->data,
> > EDITOR_EOF_PREFIX);
> > + }
>
> Move your process_comitted_list functionality *into* the
> truncate_buffer_at_prefix (renaming that function as appropriate, like
> handle_commit_paths() or something). Then make the new function do:
>
> 1. Search for the EDITOR_EOF_PREFIX. If not found, return. Else,
> remember the buffer location to "crop" to.
> 2. Handle the path list manipulations.
> 3. Crop (truncate) the buffer.
>
> No sense in pouring over this buffer twice.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
> ----------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][RESEND] Issue #1295 (take 5)
Posted by Branko Čibej <br...@xbc.nu>.
Mark Grosberg wrote:
>Anybody have a chance to look over my latest patch? Haven't heard anything
>back for several days.
>
>
I remember one relevant comment that you haven't addressed yet (see
attached message).
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/