You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/07/08 20:59:23 UTC
Re: [PATCH] Re: Adding unversioned items to the log message
Daniel Shahaf <d....@daniel.shahaf.co.il> writes:
> Ping, has anyone looked at Clint's patches? Karl, you said earlier[1]
> that you planned to do so.
I still plan to do so. I'm completely swamped in issue #2489 right now,
and can't divide my attention at the moment, but I'll try to get to this
as soon as I can.
Best,
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Adding unversioned items to the log message
Posted by Julian Foad <ju...@btopenworld.com>.
On Tue, 2008-09-23 at 22:47 +1000, Clint Lawrence wrote:
> On 23/09/2008, at 7:03 AM, Julian Foad wrote:
> > I am concerned about adding a chunk of code into the commit logic that
> > isn't closely aligned to the commit logic's task. In particular, I am
> > concerned about adding it into the library.
> >
[...]
> I was conscience of messing with the commit logic at first, but it felt
> like the path of least resistance at the time. This is my first crack
> at contributing here, so if I've taken the wrong path I'm happy try
> again.
That's great to hear! I thought I sounded a bit harsh saying that all
your good work so far wasn't the way I'd like to see it done.
> When I first looked at this, I was actually a little surprised that
> commit
> and status didn't share the same code to recurse through the wc. So I
> was
> (naively?) hoping that the commit logic would more or less have
> access to
> that information and was just throwing it away while it built the
> list of
> items to commit. I suppose thinking that shaped my approach to the
> problem.
>
> The only other advantage I can think of for putting it in the
> library, is
> that it becomes available to other users of the API. I don't think
> I'm in
> a position to judge if that would be useful or not
>
> >
> > Taking a step back, isn't the output of "svn status" what we want to
> > see? I'm not clear why we would want a special subset of it. Let me
> > give
> > some examples of what I mean:
> >
> > * Do we want to ignore items in the global-ignores and the local
> > svn:ignore list? Undoubtedly. Then we perhaps should support a
> > "--no-ignores" switch like "svn status" does. I can easily say, "No we
> > shouldn't - that's going too far," but where is the rationale behind
> > that decision?
>
> From a user interface point of view I'm not sure it would make sense
> to have a
> "--no-ignores" switch for "svn commit". I would read that to mean it
> some how
> effected what items would be committed. Of course the general point
> you raise
> is valid and I'm not sure I have a response to that at the moment.
>
> > * Where should we look for unversioned items? The present patch,
> > from
> > my quick glance, looks in each directory scanned for committables, and
> > also looks at the immediate siblings of each specified target. That's
> > fine for one user, but the next user wants to also see "cousins" like
> > "www/jpegs/unversioned.jpg" when committing "www/gifs/new.gif" as a
> > named target.
>
> Correct. That is where it looks at the moment. My inclination was to
> start
> out as simple as possible and provide something that helps in most
> cases. It
> is after all, just a reminder. How far should you go to protect
> people from
> themselves?
The point of all those questions was to illustrate that there is no
canonical answer. If we implement this feature using arbitrary answers
to those questions, it will satisfy some users but others will always be
wanting to change it in various ways. It would be better to write the
feature somewhere where technically inclined users can tweak it as they
want it.
> > * If we look for unversioned items because the user might have
> > forgotten to add them, why do we not also look for the opposite: items
> > that are "missing" because the user deleted or moved them without
> > telling Subversion?
> >
> Good idea. For myself, I'm usually forgetting to add something, but the
> symmetry is appealing.
>
> Some more constructive feedback on which direction to add would be very
> helpful for me at this point, if anyone has some to offer.
If you can, I would like to see if this can be implemented in a script
whose name is plugged in to the "SVN_EDITOR" environment variable. I
would recommend Python as a language to write it in. If you find that
plugging in to SVN_EDITOR doesn't give you access to the information you
need, discuss here and maybe we can generalise/change/extend the plug-in
support.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Adding unversioned items to the log message
Posted by Clint Lawrence <cl...@gmail.com>.
> On 23/09/2008, at 4:04 AM, Justin Erenkrantz wrote:
>> 2008/9/22 Clint Lawrence <cl...@gmail.com>:
>>> I've expanded my original patch a little and now externals and
>>> ignored
>>> items are handled correctly. The only major task that still needs
>>> to be
>>> completed is disabling the search for unversioned items when it's
>>> not
>>> required.
>>
>> Sorry for ignoring this thread until now - it just dawned on me what
>> this thread is discussing.
>>
>> Can I add in my $.02 that I would absolutely not like to see this
>> functionality added by default?
It was certainly my intention to have a way to turn this off. I don't
really
have any strong thoughts if it's better on or off by default. I will
get to
that one the basic functionality .
On 23/09/2008, at 7:03 AM, Julian Foad wrote:
> On Mon, 2008-09-22 at 22:03 +1000, Clint Lawrence wrote:
>> Updated patches are attached. The change is split into two patches -
>> libsvn_client.txt should be applied first, then client.txt.
>>
>> I've expanded my original patch a little and now externals and
>> ignored
>> items are handled correctly. The only major task that still needs
>> to be
>> completed is disabling the search for unversioned items when it's not
>> required.
>
> I am concerned about adding a chunk of code into the commit logic that
> isn't closely aligned to the commit logic's task. In particular, I am
> concerned about adding it into the library.
>
> In the existing commit message template, to generate the list of items
> to be committed, we know that the commit logic is generating that list
> anyway. If we wanted the high-level client to generate that list
> separately by using functions like "status", it would be duplicating
> code and it might not be easy to ensure it was exactly right.
> Therefore
> it makes sense for the commit logic to pass its list back to the
> high-level client code.
>
> When we want to display unversioned items in the message, I'm not sure
> any of this argument applies. Is there a single definition of
> "unversioned items" from the commit logic's point of view? I don't
> think
> so. It is a concept of the user-interface presentation logic of "svn
> status" and the preferences that control it (global-ignores, etc.).
>
> Wouldn't it make sense for us to add this feature in the user
> interface
> (client) code where it logically belongs? Is there a benefit to
> embedding it in the commit logic in the library? (There's probably at
> least a small speed advantage, but a large one?)
I was conscience of messing with the commit logic at first, but it felt
like the path of least resistance at the time. This is my first crack
at contributing here, so if I've taken the wrong path I'm happy try
again.
When I first looked at this, I was actually a little surprised that
commit
and status didn't share the same code to recurse through the wc. So I
was
(naively?) hoping that the commit logic would more or less have
access to
that information and was just throwing it away while it built the
list of
items to commit. I suppose thinking that shaped my approach to the
problem.
The only other advantage I can think of for putting it in the
library, is
that it becomes available to other users of the API. I don't think
I'm in
a position to judge if that would be useful or not
>
> Taking a step back, isn't the output of "svn status" what we want to
> see? I'm not clear why we would want a special subset of it. Let me
> give
> some examples of what I mean:
>
> * Do we want to ignore items in the global-ignores and the local
> svn:ignore list? Undoubtedly. Then we perhaps should support a
> "--no-ignores" switch like "svn status" does. I can easily say, "No we
> shouldn't - that's going too far," but where is the rationale behind
> that decision?
From a user interface point of view I'm not sure it would make sense
to have a
"--no-ignores" switch for "svn commit". I would read that to mean it
some how
effected what items would be committed. Of course the general point
you raise
is valid and I'm not sure I have a response to that at the moment.
> * Where should we look for unversioned items? The present patch,
> from
> my quick glance, looks in each directory scanned for committables, and
> also looks at the immediate siblings of each specified target. That's
> fine for one user, but the next user wants to also see "cousins" like
> "www/jpegs/unversioned.jpg" when committing "www/gifs/new.gif" as a
> named target.
Correct. That is where it looks at the moment. My inclination was to
start
out as simple as possible and provide something that helps in most
cases. It
is after all, just a reminder. How far should you go to protect
people from
themselves?
>
> * If we look for unversioned items because the user might have
> forgotten to add them, why do we not also look for the opposite: items
> that are "missing" because the user deleted or moved them without
> telling Subversion?
>
Good idea. For myself, I'm usually forgetting to add something, but the
symmetry is appealing.
Some more constructive feedback on which direction to add would be very
helpful for me at this point, if anyone has some to offer.
Cheers,
Clint
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Adding unversioned items to the log message
Posted by Julian Foad <ju...@btopenworld.com>.
On Mon, 2008-09-22 at 22:03 +1000, Clint Lawrence wrote:
> Updated patches are attached. The change is split into two patches -
> libsvn_client.txt should be applied first, then client.txt.
>
> I've expanded my original patch a little and now externals and ignored
> items are handled correctly. The only major task that still needs to be
> completed is disabling the search for unversioned items when it's not
> required.
I am concerned about adding a chunk of code into the commit logic that
isn't closely aligned to the commit logic's task. In particular, I am
concerned about adding it into the library.
In the existing commit message template, to generate the list of items
to be committed, we know that the commit logic is generating that list
anyway. If we wanted the high-level client to generate that list
separately by using functions like "status", it would be duplicating
code and it might not be easy to ensure it was exactly right. Therefore
it makes sense for the commit logic to pass its list back to the
high-level client code.
When we want to display unversioned items in the message, I'm not sure
any of this argument applies. Is there a single definition of
"unversioned items" from the commit logic's point of view? I don't think
so. It is a concept of the user-interface presentation logic of "svn
status" and the preferences that control it (global-ignores, etc.).
Wouldn't it make sense for us to add this feature in the user interface
(client) code where it logically belongs? Is there a benefit to
embedding it in the commit logic in the library? (There's probably at
least a small speed advantage, but a large one?)
Taking a step back, isn't the output of "svn status" what we want to
see? I'm not clear why we would want a special subset of it. Let me give
some examples of what I mean:
* Do we want to ignore items in the global-ignores and the local
svn:ignore list? Undoubtedly. Then we perhaps should support a
"--no-ignores" switch like "svn status" does. I can easily say, "No we
shouldn't - that's going too far," but where is the rationale behind
that decision?
* Where should we look for unversioned items? The present patch, from
my quick glance, looks in each directory scanned for committables, and
also looks at the immediate siblings of each specified target. That's
fine for one user, but the next user wants to also see "cousins" like
"www/jpegs/unversioned.jpg" when committing "www/gifs/new.gif" as a
named target.
* If we look for unversioned items because the user might have
forgotten to add them, why do we not also look for the opposite: items
that are "missing" because the user deleted or moved them without
telling Subversion?
When I write a log message, I have a little utility with which I insert
a skeleton log message and a diff into my log message text [1], and then
I edit this and finally delete the diff lines before saving it. If I
wanted to see what files I'd forgotten, I would simply insert the output
of "svn status ." or "svn status COMMIT_TARGETS" into my text. Now, I
know that many popular editors don't make it easy to do that, but it
would feel right for us to provide some assistance for doing that in
Subversion's external log message editor program hook rather than
building in more special code.
- Julian
[1] The attached shell script "svnlogmsg".
Re: [PATCH] Re: Adding unversioned items to the log message
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
2008/9/22 Clint Lawrence <cl...@gmail.com>:
> I've expanded my original patch a little and now externals and ignored
> items are handled correctly. The only major task that still needs to be
> completed is disabling the search for unversioned items when it's not
> required.
Sorry for ignoring this thread until now - it just dawned on me what
this thread is discussing.
Can I add in my $.02 that I would absolutely not like to see this
functionality added by default? I have *lots* of unversioned (and
unignored) files in a lot of my WCs. I do not want to see my log
message template get consistently cluttered with that information.
For example, *just* in my Subversion WC, I have 31 '?' files - most of
these are patches and other notes to myself. (For $WORK, I have 120
such files in my main working copy directory.)
In the issue, Ben's example of TortoiseSVN being a good example of the
worthiness of always including this is something I disagree with -
TortoiseSVN is a UI that lets you change or modify the committable
list while you are writing the log message. When we invoke
$SVN_EDITOR, we don't have that ability (though we have talked about
it before). Plus, TortoiseSVN has the functionality to hit a checkbox
and all of the unversioned items go away. Guess what setting I use
with TortoiseSVN? *grin*
Therefore, if we have to add this, can we please have a knob that
turns this feature on (or off, if we do decide that including it is
best)? -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Adding unversioned items to the log message
Posted by Clint Lawrence <cl...@gmail.com>.
On 15/09/2008, at 11:55 PM, Daniel Shahaf wrote:
> Clint Lawrence wrote on Tue, 22 Jul 2008 at 07:40 +0100:
>> On 08/07/2008, at 9:59 PM, Karl Fogel wrote:
>>
>>> Daniel Shahaf <d....@daniel.shahaf.co.il> writes:
>>>> Ping, has anyone looked at Clint's patches? Karl, you said
>>>> earlier[1]
>>>> that you planned to do so.
>>>
>>> I still plan to do so. I'm completely swamped in issue #2489
>>> right now,
>>> and can't divide my attention at the moment, but I'll try to get
>>> to this
>>> as soon as I can.
>>>
>>> Best,
>>> -Karl
>>
>> Thanks Karl - Good to know it's not forgotten. My original patch
>> conflicts with the current trunk now, so I'll try to fix that up and
>> repost sometime soon.
>
> Ping, I haven't seen the repost?
Updated patches are attached. The change is split into two patches -
libsvn_client.txt should be applied first, then client.txt.
I've expanded my original patch a little and now externals and ignored
items are handled correctly. The only major task that still needs to be
completed is disabling the search for unversioned items when it's not
required.
Cheers,
Clint
Re: [PATCH] Re: Adding unversioned items to the log message
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Clint Lawrence wrote on Tue, 22 Jul 2008 at 07:40 +0100:
> On 08/07/2008, at 9:59 PM, Karl Fogel wrote:
>
> > Daniel Shahaf <d....@daniel.shahaf.co.il> writes:
> > > Ping, has anyone looked at Clint's patches? Karl, you said earlier[1]
> > > that you planned to do so.
> >
> > I still plan to do so. I'm completely swamped in issue #2489 right now,
> > and can't divide my attention at the moment, but I'll try to get to this
> > as soon as I can.
> >
> > Best,
> > -Karl
>
> Thanks Karl - Good to know it's not forgotten. My original patch
> conflicts with the current trunk now, so I'll try to fix that up and
> repost sometime soon.
Ping, I haven't seen the repost?
> I'm in no big hurry so whenever you can slot it
> in works for me.
>
> Cheers, Clint
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Adding unversioned items to the log message
Posted by Clint Lawrence <cl...@gmail.com>.
On 08/07/2008, at 9:59 PM, Karl Fogel wrote:
> Daniel Shahaf <d....@daniel.shahaf.co.il> writes:
>> Ping, has anyone looked at Clint's patches? Karl, you said earlier
>> [1]
>> that you planned to do so.
>
> I still plan to do so. I'm completely swamped in issue #2489 right
> now,
> and can't divide my attention at the moment, but I'll try to get to
> this
> as soon as I can.
>
> Best,
> -Karl
Thanks Karl - Good to know it's not forgotten. My original patch
conflicts with
the current trunk now, so I'll try to fix that up and repost sometime
soon. I'm
in no big hurry so whenever you can slot it in works for me.
Cheers,
Clint
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org