Re: #448

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: #448

Kohsuke Kawaguchi
Administrator
Valeri Felberg wrote:

> On 8/7/07, Kohsuke Kawaguchi <[hidden email]> wrote:
>>
>> I was thinking of #448 as just the equivalent of the Unix "find"
>> command. Since files in the workspace is changing all the time, I
>> thought indexing them with Lucene wouldn't make much sense.
>
>
> I think you are right: it would be an overkill to update the index each time
> the workspace changes. Search for a file occurs presumably much less often
> and is not as time-critical as, say, an SCM update during a build. So, it
> could be implemented as a simple tree traversal.
>
> I would try to reuse/extend the input box in the workspace view to accept
> Ant file/directory patterns like **/*.xml which would be interpreted
> relative to the current workspace directory. The current output could be
> just replicated to display the matched files in different subdirectories.
+1. That's exactly what I had in mind.


--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: #448

Valeri Felberg
I send a patch for the issue, first, because I could not check out the source as vfelberg (CVS says: no such user) and, second, because may be you want to review the changes first. Here are some of them which may require an explanation:

dir.jelly
- form input renamed into "pattern" so that the request attributes do not get mixed up
- form method changed to post: in case of pattern set there is no redirect now so that the pattern remains in the url. Was there an important reason for the get method?
- file icon name defined by the last name in the path list instead of the first so that the matched files have the correct icon

FilePath.length() added delegating to the corresponding File: just a convenience method similar to exists()

The bulk of changes is in DirectoryBrowserSupport but these are too many to list. Check it out and feel free to criticize :)

On 8/8/07, Kohsuke Kawaguchi <[hidden email]> wrote:
Valeri Felberg wrote:

> On 8/7/07, Kohsuke Kawaguchi < [hidden email]> wrote:
>>
>> I was thinking of #448 as just the equivalent of the Unix "find"
>> command. Since files in the workspace is changing all the time, I
>> thought indexing them with Lucene wouldn't make much sense.
>
>
> I think you are right: it would be an overkill to update the index each time
> the workspace changes. Search for a file occurs presumably much less often
> and is not as time-critical as, say, an SCM update during a build. So, it
> could be implemented as a simple tree traversal.
>
> I would try to reuse/extend the input box in the workspace view to accept
> Ant file/directory patterns like **/*.xml which would be interpreted
> relative to the current workspace directory. The current output could be
> just replicated to display the matched files in different subdirectories.

+1. That's exactly what I had in mind.


--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

patch-448.txt (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: #448

Kohsuke Kawaguchi
Administrator

The patch looks good. I applied the patch and committed the changes.

Valeri Felberg wrote:
> I send a patch for the issue, first, because I could not check out the
> source as vfelberg (CVS says: no such user) and, second, because may be you
> want to review the changes first. Here are some of them which may require an
> explanation:

Can you show me the exact cvs command you typed? I do see the user id
'vfelberg' with developer access.

>
> dir.jelly
> - form input renamed into "pattern" so that the request attributes do not
> get mixed up
> - form method changed to post: in case of pattern set there is no redirect
> now so that the pattern remains in the url. Was there an important reason
> for the get method?

Yes. The reason 'get' was used is so that the user can share&bookmark
the result page. So I changed it back to get.

> - file icon name defined by the last name in the path list instead of the
> first so that the matched files have the correct icon
>
> FilePath.length() added delegating to the corresponding File: just a
> convenience method similar to exists()
>
> The bulk of changes is in DirectoryBrowserSupport but these are too many to
> list. Check it out and feel free to criticize :)
>
> On 8/8/07, Kohsuke Kawaguchi < [hidden email]> wrote:
>>
>> Valeri Felberg wrote:
>> > On 8/7/07, Kohsuke Kawaguchi < [hidden email]> wrote:
>> >>
>> >> I was thinking of #448 as just the equivalent of the Unix "find"
>> >> command. Since files in the workspace is changing all the time, I
>> >> thought indexing them with Lucene wouldn't make much sense.
>> >
>> >
>> > I think you are right: it would be an overkill to update the index each
>> time
>> > the workspace changes. Search for a file occurs presumably much less
>> often
>> > and is not as time-critical as, say, an SCM update during a build. So,
>> it
>> > could be implemented as a simple tree traversal.
>> >
>> > I would try to reuse/extend the input box in the workspace view to
>> accept
>> > Ant file/directory patterns like **/*.xml which would be interpreted
>> > relative to the current workspace directory. The current output could be
>> > just replicated to display the matched files in different
>> subdirectories.
>>
>> +1. That's exactly what I had in mind.
>>
>>
>> --
>> Kohsuke Kawaguchi
>> Sun Microsystems                   [hidden email]
>>
>>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]

--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: #448

Valeri Felberg
On 8/13/07, Kohsuke Kawaguchi <[hidden email]> wrote:

The patch looks good. I applied the patch and committed the changes.

Valeri Felberg wrote:
> I send a patch for the issue, first, because I could not check out the
> source as vfelberg (CVS says: no such user) and, second, because may be you
> want to review the changes first. Here are some of them which may require an
> explanation:

Can you show me the exact cvs command you typed? I do see the user id
'vfelberg' with developer access.

cvs -d :pserver:[hidden email]:/cvs co hudson/hudson

I see my user name in the project's member list with roles observer and developer but CVS says: no such user.

>
> dir.jelly
> - form input renamed into "pattern" so that the request attributes do not
> get mixed up
> - form method changed to post: in case of pattern set there is no redirect
> now so that the pattern remains in the url. Was there an important reason
> for the get method?

Yes. The reason 'get' was used is so that the user can share&bookmark
the result page. So I changed it back to get.

Ah, I see. So I yet have to remove the query portion from the links on the result page so that one can navigate to the parent folders of the matched files.

I forgot to mention one difference in the behavior of the input box. Before, you could navigate to a subfolder of the current directory by typing its relative path into the box. Now you can only search for files (not directories) with it since Ant's DirectoryScanner only matches files (as far as I could see). If you want to search for subdirectories, I could try to find a way.

> - file icon name defined by the last name in the path list instead of the

> first so that the matched files have the correct icon
>
> FilePath.length() added delegating to the corresponding File: just a
> convenience method similar to exists()
>
> The bulk of changes is in DirectoryBrowserSupport but these are too many to
> list. Check it out and feel free to criticize :)
>
> On 8/8/07, Kohsuke Kawaguchi < [hidden email]> wrote:
>>
>> Valeri Felberg wrote:
>> > On 8/7/07, Kohsuke Kawaguchi < [hidden email]> wrote:
>> >>
>> >> I was thinking of #448 as just the equivalent of the Unix "find"
>> >> command. Since files in the workspace is changing all the time, I
>> >> thought indexing them with Lucene wouldn't make much sense.
>> >
>> >
>> > I think you are right: it would be an overkill to update the index each
>> time
>> > the workspace changes. Search for a file occurs presumably much less
>> often
>> > and is not as time-critical as, say, an SCM update during a build. So,
>> it
>> > could be implemented as a simple tree traversal.
>> >
>> > I would try to reuse/extend the input box in the workspace view to
>> accept
>> > Ant file/directory patterns like **/*.xml which would be interpreted
>> > relative to the current workspace directory. The current output could be
>> > just replicated to display the matched files in different
>> subdirectories.
>>
>> +1. That's exactly what I had in mind.
>>
>>
>> --
>> Kohsuke Kawaguchi
>> Sun Microsystems                   [hidden email]
>>
>>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]


--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: #448

Peter Reilly-2
On 8/13/07, Valeri Felberg <[hidden email]> wrote:

> On 8/13/07, Kohsuke Kawaguchi <[hidden email]> wrote:
> >
> > The patch looks good. I applied the patch and committed the changes.
> >
> > Valeri Felberg wrote:
> > > I send a patch for the issue, first, because I could not check out the
> > > source as vfelberg (CVS says: no such user) and, second, because may be
> you
> > > want to review the changes first. Here are some of them which may
> require an
> > > explanation:
> >
> > Can you show me the exact cvs command you typed? I do see the user id
> > 'vfelberg' with developer access.
>
> cvs -d :pserver:[hidden email]:/cvs co
> hudson/hudson
>
> I see my user name in the project's member list with roles observer and
> developer but CVS says: no such user.

I had the same problem, I had to login on cvs first.
cvs -d :pserver:[hidden email]:/cvs login

Peter

>
>
> > >
> > > dir.jelly
> > > - form input renamed into "pattern" so that the request attributes do
> not
> > > get mixed up
> > > - form method changed to post: in case of pattern set there is no
> redirect
> > > now so that the pattern remains in the url. Was there an important
> reason
> > > for the get method?
> >
> > Yes. The reason 'get' was used is so that the user can share&bookmark
> > the result page. So I changed it back to get.
>
> Ah, I see. So I yet have to remove the query portion from the links on the
> result page so that one can navigate to the parent folders of the matched
> files.
>
> I forgot to mention one difference in the behavior of the input box. Before,
> you could navigate to a subfolder of the current directory by typing its
> relative path into the box. Now you can only search for files (not
> directories) with it since Ant's DirectoryScanner only matches files (as far
> as I could see). If you want to search for subdirectories, I could try to
> find a way.
>
> > > - file icon name defined by the last name in the path list instead of
> the
> > > first so that the matched files have the correct icon
> > >
> > > FilePath.length() added delegating to the corresponding File: just a
> > > convenience method similar to exists()
> > >
> > > The bulk of changes is in DirectoryBrowserSupport but these are too many
> to
> > > list. Check it out and feel free to criticize :)
> > >
> > > On 8/8/07, Kohsuke Kawaguchi < [hidden email]> wrote:
> > >>
> > >> Valeri Felberg wrote:
> > >> > On 8/7/07, Kohsuke Kawaguchi < [hidden email]> wrote:
> > >> >>
> > >> >> I was thinking of #448 as just the equivalent of the Unix "find"
> > >> >> command. Since files in the workspace is changing all the time, I
> > >> >> thought indexing them with Lucene wouldn't make much sense.
> > >> >
> > >> >
> > >> > I think you are right: it would be an overkill to update the index
> each
> > >> time
> > >> > the workspace changes. Search for a file occurs presumably much less
> > >> often
> > >> > and is not as time-critical as, say, an SCM update during a build.
> So,
> > >> it
> > >> > could be implemented as a simple tree traversal.
> > >> >
> > >> > I would try to reuse/extend the input box in the workspace view to
> > >> accept
> > >> > Ant file/directory patterns like **/*.xml which would be interpreted
> > >> > relative to the current workspace directory. The current output could
> be
> > >> > just replicated to display the matched files in different
> > >> subdirectories.
> > >>
> > >> +1. That's exactly what I had in mind.
> > >>
> > >>
> > >> --
> > >> Kohsuke Kawaguchi
> > >> Sun Microsystems                   [hidden email]
> > >>
> > >>
> > >
> > >
> > >
> ------------------------------------------------------------------------
> > >
> > >
> ---------------------------------------------------------------------
> > > To unsubscribe, e-mail:
> [hidden email]
> > > For additional commands, e-mail: [hidden email]
> >
> >
> > --
> > Kohsuke Kawaguchi
> > Sun Microsystems                   [hidden email]
> >
> >
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: #448

Valeri Felberg
On 8/13/07, Peter Reilly <[hidden email]> wrote:
On 8/13/07, Valeri Felberg <[hidden email]> wrote:

> On 8/13/07, Kohsuke Kawaguchi <[hidden email]> wrote:
> >
> > The patch looks good. I applied the patch and committed the changes.
> >
> > Valeri Felberg wrote:
> > > I send a patch for the issue, first, because I could not check out the
> > > source as vfelberg (CVS says: no such user) and, second, because may be
> you
> > > want to review the changes first. Here are some of them which may
> require an
> > > explanation:
> >
> > Can you show me the exact cvs command you typed? I do see the user id
> > 'vfelberg' with developer access.
>
> cvs -d :pserver:[hidden email]:/cvs co
> hudson/hudson
>
> I see my user name in the project's member list with roles observer and
> developer but CVS says: no such user.

I had the same problem, I had to login on cvs first.
cvs -d : pserver:[hidden email]:/cvs login

Ah, thanks, I'll try it out.

Peter

>
>
> > >
> > > dir.jelly
> > > - form input renamed into "pattern" so that the request attributes do
> not
> > > get mixed up
> > > - form method changed to post: in case of pattern set there is no
> redirect
> > > now so that the pattern remains in the url. Was there an important
> reason
> > > for the get method?
> >
> > Yes. The reason 'get' was used is so that the user can share&bookmark
> > the result page. So I changed it back to get.
>
> Ah, I see. So I yet have to remove the query portion from the links on the
> result page so that one can navigate to the parent folders of the matched
> files.
>
> I forgot to mention one difference in the behavior of the input box. Before,
> you could navigate to a subfolder of the current directory by typing its
> relative path into the box. Now you can only search for files (not
> directories) with it since Ant's DirectoryScanner only matches files (as far
> as I could see). If you want to search for subdirectories, I could try to
> find a way.
>
> > > - file icon name defined by the last name in the path list instead of
> the
> > > first so that the matched files have the correct icon
> > >
> > > FilePath.length() added delegating to the corresponding File: just a
> > > convenience method similar to exists()
> > >
> > > The bulk of changes is in DirectoryBrowserSupport but these are too many
> to
> > > list. Check it out and feel free to criticize :)
> > >
> > > On 8/8/07, Kohsuke Kawaguchi < [hidden email]> wrote:
> > >>
> > >> Valeri Felberg wrote:
> > >> > On 8/7/07, Kohsuke Kawaguchi < [hidden email]> wrote:
> > >> >>
> > >> >> I was thinking of #448 as just the equivalent of the Unix "find"
> > >> >> command. Since files in the workspace is changing all the time, I
> > >> >> thought indexing them with Lucene wouldn't make much sense.
> > >> >
> > >> >
> > >> > I think you are right: it would be an overkill to update the index
> each
> > >> time
> > >> > the workspace changes. Search for a file occurs presumably much less
> > >> often
> > >> > and is not as time-critical as, say, an SCM update during a build.
> So,
> > >> it
> > >> > could be implemented as a simple tree traversal.
> > >> >
> > >> > I would try to reuse/extend the input box in the workspace view to
> > >> accept
> > >> > Ant file/directory patterns like **/*.xml which would be interpreted
> > >> > relative to the current workspace directory. The current output could
> be
> > >> > just replicated to display the matched files in different
> > >> subdirectories.
> > >>
> > >> +1. That's exactly what I had in mind.
> > >>
> > >>
> > >> --
> > >> Kohsuke Kawaguchi
> > >> Sun Microsystems                   [hidden email]
> > >>
> > >>
> > >
> > >
> > >
> ------------------------------------------------------------------------
> > >
> > >
> ---------------------------------------------------------------------
> > > To unsubscribe, e-mail:
> [hidden email]
> > > For additional commands, e-mail: [hidden email]
> >
> >
> > --
> > Kohsuke Kawaguchi
> > Sun Microsystems                   [hidden email]
> >
> >
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: #448

Valeri Felberg
On 8/13/07, Valeri Felberg <[hidden email]> wrote:
On 8/13/07, Peter Reilly <[hidden email] > wrote:
On 8/13/07, Valeri Felberg <[hidden email]> wrote:

> On 8/13/07, Kohsuke Kawaguchi < [hidden email]> wrote:
> >
> > The patch looks good. I applied the patch and committed the changes.
> >
> > Valeri Felberg wrote:
> > > I send a patch for the issue, first, because I could not check out the
> > > source as vfelberg (CVS says: no such user) and, second, because may be
> you
> > > want to review the changes first. Here are some of them which may
> require an
> > > explanation:
> >
> > Can you show me the exact cvs command you typed? I do see the user id
> > 'vfelberg' with developer access.
>
> cvs -d :pserver:[hidden email]:/cvs co
> hudson/hudson
>
> I see my user name in the project's member list with roles observer and
> developer but CVS says: no such user.

I had the same problem, I had to login on cvs first.
cvs -d : pserver:[hidden email]:/cvs login

Ah, thanks, I'll try it out.

Yeah, it works, thanks again. May be it should be mentioned somewhere under "Building Hudson" in the Wiki?

Peter

>
>
> > >
> > > dir.jelly
> > > - form input renamed into "pattern" so that the request attributes do
> not
> > > get mixed up
> > > - form method changed to post: in case of pattern set there is no
> redirect
> > > now so that the pattern remains in the url. Was there an important
> reason
> > > for the get method?
> >
> > Yes. The reason 'get' was used is so that the user can share&bookmark
> > the result page. So I changed it back to get.
>
> Ah, I see. So I yet have to remove the query portion from the links on the
> result page so that one can navigate to the parent folders of the matched
> files.
>
> I forgot to mention one difference in the behavior of the input box. Before,
> you could navigate to a subfolder of the current directory by typing its
> relative path into the box. Now you can only search for files (not
> directories) with it since Ant's DirectoryScanner only matches files (as far
> as I could see). If you want to search for subdirectories, I could try to
> find a way.
>
> > > - file icon name defined by the last name in the path list instead of
> the
> > > first so that the matched files have the correct icon
> > >
> > > FilePath.length() added delegating to the corresponding File: just a
> > > convenience method similar to exists()
> > >
> > > The bulk of changes is in DirectoryBrowserSupport but these are too many
> to
> > > list. Check it out and feel free to criticize :)
> > >
> > > On 8/8/07, Kohsuke Kawaguchi < [hidden email]> wrote:
> > >>
> > >> Valeri Felberg wrote:
> > >> > On 8/7/07, Kohsuke Kawaguchi < [hidden email]> wrote:
> > >> >>
> > >> >> I was thinking of #448 as just the equivalent of the Unix "find"
> > >> >> command. Since files in the workspace is changing all the time, I
> > >> >> thought indexing them with Lucene wouldn't make much sense.
> > >> >
> > >> >
> > >> > I think you are right: it would be an overkill to update the index
> each
> > >> time
> > >> > the workspace changes. Search for a file occurs presumably much less
> > >> often
> > >> > and is not as time-critical as, say, an SCM update during a build.
> So,
> > >> it
> > >> > could be implemented as a simple tree traversal.
> > >> >
> > >> > I would try to reuse/extend the input box in the workspace view to
> > >> accept
> > >> > Ant file/directory patterns like **/*.xml which would be interpreted
> > >> > relative to the current workspace directory. The current output could
> be
> > >> > just replicated to display the matched files in different
> > >> subdirectories.
> > >>
> > >> +1. That's exactly what I had in mind.
> > >>
> > >>
> > >> --
> > >> Kohsuke Kawaguchi
> > >> Sun Microsystems                   [hidden email]
> > >>
> > >>
> > >
> > >
> > >
> ------------------------------------------------------------------------
> > >
> > >
> ---------------------------------------------------------------------
> > > To unsubscribe, e-mail:
> [hidden email]
> > > For additional commands, e-mail: [hidden email]
> >
> >
> > --
> > Kohsuke Kawaguchi
> > Sun Microsystems                   [hidden email]
> >
> >
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]



Reply | Threaded
Open this post in threaded view
|

Re: #448

Kohsuke Kawaguchi
Administrator
Valeri Felberg wrote:

> On 8/13/07, Valeri Felberg <[hidden email]> wrote:
>>
>> On 8/13/07, Peter Reilly <[hidden email]> wrote:
>> >
>> > On 8/13/07, Valeri Felberg <[hidden email]> wrote:
>> > > On 8/13/07, Kohsuke Kawaguchi <[hidden email] > wrote:
>> > > >
>> > > > The patch looks good. I applied the patch and committed the changes.
>> > > >
>> > > > Valeri Felberg wrote:
>> > > > > I send a patch for the issue, first, because I could not check out
>> > the
>> > > > > source as vfelberg (CVS says: no such user) and, second, because
>> > may be
>> > > you
>> > > > > want to review the changes first. Here are some of them which may
>> > > require an
>> > > > > explanation:
>> > > >
>> > > > Can you show me the exact cvs command you typed? I do see the user
>> > id
>> > > > 'vfelberg' with developer access.
>> > >
>> > > cvs -d :pserver:[hidden email]:/cvs co
>> > > hudson/hudson
>> > >
>> > > I see my user name in the project's member list with roles observer
>> > and
>> > > developer but CVS says: no such user.
>> >
>> > I had the same problem, I had to login on cvs first.
>> > cvs -d : pserver:[hidden email]:/cvs login
>>
>>
>> Ah, thanks, I'll try it out.
>>
>
> Yeah, it works, thanks again. May be it should be mentioned somewhere under
> "Building Hudson" in the Wiki?
Yes, please!


--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: #448

Valeri Felberg
On 8/15/07, Kohsuke Kawaguchi <[hidden email]> wrote:
Valeri Felberg wrote:

> On 8/13/07, Valeri Felberg <[hidden email]> wrote:
>>
>> On 8/13/07, Peter Reilly <[hidden email]> wrote:
>> >
>> > On 8/13/07, Valeri Felberg <[hidden email]> wrote:
>> > > On 8/13/07, Kohsuke Kawaguchi < [hidden email] > wrote:
>> > > >
>> > > > The patch looks good. I applied the patch and committed the changes.
>> > > >
>> > > > Valeri Felberg wrote:
>> > > > > I send a patch for the issue, first, because I could not check out
>> > the
>> > > > > source as vfelberg (CVS says: no such user) and, second, because
>> > may be
>> > > you
>> > > > > want to review the changes first. Here are some of them which may
>> > > require an
>> > > > > explanation:
>> > > >
>> > > > Can you show me the exact cvs command you typed? I do see the user
>> > id
>> > > > 'vfelberg' with developer access.
>> > >
>> > > cvs -d :pserver:[hidden email]:/cvs co
>> > > hudson/hudson
>> > >
>> > > I see my user name in the project's member list with roles observer
>> > and
>> > > developer but CVS says: no such user.
>> >
>> > I had the same problem, I had to login on cvs first.
>> > cvs -d : pserver:[hidden email] :/cvs login
>>
>>
>> Ah, thanks, I'll try it out.
>>
>
> Yeah, it works, thanks again. May be it should be mentioned somewhere under
> "Building Hudson" in the Wiki?

Yes, please!

Done.

--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]