You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Joshua Varner <jl...@gmail.com> on 2005/06/08 19:51:06 UTC
[PATCH] Unexpected behavior from psvn.el
I was adding svn:ignore properties to my project and wanted to check
those in, but there were some other local changes that i didn't want
commited. So using psvn.el I marked the directories (which
automatically marks any changed files under that directory), then
unmarked those changed files and ran the commit. To my surprise the
unmarked changed files were committed anyway.
After digging a little I saw what was needed was a -N to avoid the
recursion, but there was no way to do that in psvn. But i realized
that since marking automatically selects all the files under a
directory anyway, all of those files should be explicitly listed on
the command. So the only time you would have a directory marked but
not a file under it is if you explicitly unmarked the file.
So what i'm trying to say in a rambling manner, is that I think -N
should be default for psvn. I tested the normal case (when you want
the files under a directory committed and as long as they are marked
this change functions the same as before). Patch below
Thanks,
Josh
[[
psvn.el(svn-log-edit-done) Added -N to commit command
]]
Index: psvn.el
===================================================================
--- psvn.el (revision 15011)
+++ psvn.el (working copy)
@@ -2965,7 +2965,7 @@
(svn-status-create-arg-file svn-status-temp-arg-file ""
svn-status-files-to-commit "")
(svn-run-svn t t 'commit "commit" "--targets" svn-status-temp-arg-file
- "-F" svn-status-temp-file-to-remove))
+ "-N" "-F" svn-status-temp-file-to-remove))
(set-window-configuration svn-status-pre-commit-window-configuration)
(message "svn-log editing done")))
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Unexpected behavior from psvn.el
Posted by Kalle Olavi Niemitalo <ko...@iki.fi>.
Joshua Varner <jl...@gmail.com> writes:
> I'll make a new patch, it will just take some time -- I want to get
> more comfortable with the code for the mode before making a more
> extensive patch. I'm, also, on 12 hour days at work until the 27th so
> that will cause a delay. But it is on my TODO list.
Don't bother, I made a new patch myself. This is for revision
15394 and I don't know whether it works correctly with r15539.
I am nowadays mostly offline (due to lightning damage) so it
would be good if someone else (Stefan Reichör?) could test this
with XEmacs and perhaps commit.
There is a known bug: this code assumes that svn diff
-non-recursive DIRECTORY shows diffs only for (the properties of)
the directory itself, but it actually shows diffs also for the
files in the directory. Thus, if you have marked a directory
(and thus normally also the files in it), you get the same diffs
twice. I don't yet know what would be the best way to fix this.
Possibilities include:
(a) Change svn diff --non-recursive to diff only the directory.
(b) Define a new option that diffs only the directory.
(c) Make psvn remove the extra diffs itself when it calls svn
diff --non-recursive on a directory. This wastes CPU time,
although perhaps not too much.
(d) Make psvn detect duplicate diffs in the concatenated output
and filter them out. This too wastes CPU time and would
still display diffs for the files even if only the directory
was marked; but perhaps users prefer that.
(e) Put both the directory and the files as arguments for the
same svn diff command and hope that it won't repeat the
diffs. I don't know whether this actually works, and anyway
it too would still display diffs for the files even if only
the directory was marked.
(f) Document the bug as a feature.
In the short run, I think I'd prefer option (c); it can later be
switched to (b) with no change in the user interface, if the svn
committee agrees.
I have also been working on some other changes to psvn, but I'll
post them in a separate thread.
Re: [PATCH] Unexpected behavior from psvn.el
Posted by Joshua Varner <jl...@gmail.com>.
On 7/8/05, Kalle Olavi Niemitalo <ko...@iki.fi> wrote:
>
> Could you (Varner) post a new patch with these changes? I'd then
> test it some time next week and apply it unless xsteve vetoes it.
>
I'll make a new patch, it will just take some time -- I want to get
more comfortable with the code for the mode before making a more
extensive patch. I'm, also, on 12 hour days at work until the 27th so
that will cause a delay. But it is on my TODO list.
Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Unexpected behavior from psvn.el
Posted by Kalle Olavi Niemitalo <ko...@iki.fi>.
Michael W Thelen <mi...@pietdepsi.com> writes:
> Michael W Thelen wrote:
>> Thank you for the patch... since psvn.el is in the contrib area, and I
>> believe Stefan Reichör (xsteve) is mainly responsible for maintaining
>> it, we may want his input on whether this change should be made. Stefan
>> and any other interested developers, what do you think?
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=2357
I think it would be a good change wrt svn commit. However:
* svn-status-add-file already uses --non-recursive, so I think
svn-log-edit-done should also use that, rather than -N.
* I'd rather not have commit treat the list of marked files
differently from other commands, unless there is a specific
reason. The current list (as of r15300 in trunk) seems to be:
- svn add (svn-status-add-file-recursively,
svn-status-add-file): both options available
- svn blame (svn-status-blame): no such options
- svn cat (svn-status-get-specific-revision-internal): no such
options
- svn cleanup (svn-status-cleanup): no such options
- svn commit (svn-log-edit-done): recursive by default, but the
patch would change this
- svn diff (svn-status-show-svn-diff-internal): recursive by
default; (svn-log-view-diff): recursive by default but
ignores marks
- svn info (svn-status-parse-info, svn-status-info): not
recursive by default
- svn log (svn-status-show-svn-log): no such options
- svn mkdir (svn-status-make-directory): no such options
- svn mv (svn-status-mv): no such options
- svn propdel (svn-status-property-parse-property-names): not
recursive by default
- svn propget (svn-status-property-edit,
svn-log-edit-log-entry): not recursive by default
- svn proplist (svn-status-property-list,
svn-status-proplist-start): no such options
- svn propset (svn-status-property-parse-property-names,
svn-prop-edit-do-it, svn-log-edit-done): not recursive by
default
- svn resolved (svn-status-resolved): not recursive by default
- svn revert (svn-status-revert): not recursive by default
- svn rm (svn-status-rm): no such options
- svn status (svn-status): recursive, but ignores marks
- svn update (svn-status-update-cmd): recursive by default,
but ignores marks
So, the question is whether svn diff of marked files should be
--non-recursive. I think it should, because I remember having
gotten duplicate diffs due to marking both a directory and the
files in it.
Could you (Varner) post a new patch with these changes? I'd then
test it some time next week and apply it unless xsteve vetoes it.
Re: [PATCH] Unexpected behavior from psvn.el
Posted by Michael W Thelen <mi...@pietdepsi.com>.
Michael W Thelen wrote:
>>So what i'm trying to say in a rambling manner, is that I think -N
>>should be default for psvn. I tested the normal case (when you want
>>the files under a directory committed and as long as they are marked
>>this change functions the same as before). Patch below
>
> Thank you for the patch... since psvn.el is in the contrib area, and I
> believe Stefan Reichör (xsteve) is mainly responsible for maintaining
> it, we may want his input on whether this change should be made. Stefan
> and any other interested developers, what do you think?
>
> If no one responds in a day or two, I'll file this patch in the issue
> tracker so it's not forgotten.
Thanks again for the patch. I've (finally) filed it as issue #2357.
I'm sorry it took me so long. Long story made short, I went on vacation
and my hard drive died! Here's a link to the issue:
http://subversion.tigris.org/issues/show_bug.cgi?id=2357
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Unexpected behavior from psvn.el
Posted by Michael W Thelen <mi...@pietdepsi.com>.
Joshua Varner wrote:
> So what i'm trying to say in a rambling manner, is that I think -N
> should be default for psvn. I tested the normal case (when you want
> the files under a directory committed and as long as they are marked
> this change functions the same as before). Patch below
Thank you for the patch... since psvn.el is in the contrib area, and I
believe Stefan Reichör (xsteve) is mainly responsible for maintaining
it, we may want his input on whether this change should be made. Stefan
and any other interested developers, what do you think?
If no one responds in a day or two, I'll file this patch in the issue
tracker so it's not forgotten.
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org