Codestriker: collaborative code reviewer

Hi Vlad,

> I'm creating topics directly from Subversion. One problem I've noticed
> immediately
> is that file list in a topic is not sorted by filename. Instead, files
> are listed in some
> semi-sorted order defined by Subversion. I couldn't find any
> configuration options
> that would trigger file sorting. Did I miss something?

The order the files are presented is the same as that in the text 
uploaded into Codestriker, or the order obtained from the SCM, in your 
case, Subversion.  So no, you didn't miss anything.

> Patching the code to sort by filename was rather trivial: in
> lib\Codestriker\FileParser\Parser.pm,
> insert just before the first return:
> 
>     # Sort the diff chunks by filename, then old linenumber
>     @diffs = sort { $a->{filename} cmp $b->{filename} ||
> $a->{old_linenumber} <=> $b->{old_linenumber} } @diffs;
>  
> Would it make sense to make this a configurable feature?

I can imagine this could be useful for large reviews.  Rather than 
making this a configurable feature (codestriker.conf is also quite large 
and complex), it might be useful to have a link instead on the view 
topic page above the list of files, for example:

Download topic text | ... | Tab width ... | Sort by filename

Clicking on the sort by filename link will set a CGI parameter, which 
will then execute code similar to what you have, but this would be in 
ViewTopic.pm over the list of delta objects.  That way, a reviewer can 
choose at runtime which way they'd prefer to see the list of files 
presented.

What do you think?

Cheers,
David

Hi,

How about it just sorts it all of the time given that the order that svn returns the files really
does not have any meaning.

Thanks
Jason

--- David Sitsky <sits@...> wrote:

> Hi Vlad,
> 
> > I'm creating topics directly from Subversion. One problem I've noticed
> > immediately
> > is that file list in a topic is not sorted by filename. Instead, files
> > are listed in some
> > semi-sorted order defined by Subversion. I couldn't find any
> > configuration options
> > that would trigger file sorting. Did I miss something?
> 
> The order the files are presented is the same as that in the text 
> uploaded into Codestriker, or the order obtained from the SCM, in your 
> case, Subversion.  So no, you didn't miss anything.
> 
> > Patching the code to sort by filename was rather trivial: in
> > lib\Codestriker\FileParser\Parser.pm,
> > insert just before the first return:
> > 
> >     # Sort the diff chunks by filename, then old linenumber
> >     @diffs = sort { $a->{filename} cmp $b->{filename} ||
> > $a->{old_linenumber} <=> $b->{old_linenumber} } @diffs;
> >  
> > Would it make sense to make this a configurable feature?
> 
> I can imagine this could be useful for large reviews.  Rather than 
> making this a configurable feature (codestriker.conf is also quite large 
> and complex), it might be useful to have a link instead on the view 
> topic page above the list of files, for example:
> 
> Download topic text | ... | Tab width ... | Sort by filename
> 
> Clicking on the sort by filename link will set a CGI parameter, which 
> will then execute code similar to what you have, but this would be in 
> ViewTopic.pm over the list of delta objects.  That way, a reviewer can 
> choose at runtime which way they'd prefer to see the list of files 
> presented.
> 
> What do you think?
> 
> Cheers,
> David
> 
> 
> 
> -------------------------------------------------------
> All the advantages of Linux Managed Hosting--Without the Cost and Risk!
> Fully trained technicians. The highest number of Red Hat certifications in
> the hosting industry. Fanatical Support. Click to learn more
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
> _______________________________________________
> Codestriker-user mailing list
> Codestriker-user@...
> https://lists.sourceforge.net/lists/listinfo/codestriker-user
> 


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com

Sure.. my only concern was the use case where somebody might assemble 
the topic text "by hand" in a specific order, which we would then override.

But perhaps this is a pretty obscure use case (I've never done it).  Its 
actually interesting Subversion doesn't produce the files in sorted 
order, as I believe CVS diffs are.

Ok - if nobody has an objections, I'll commit Vlad's code in.

Cheers,
David

Jason Remillard wrote:
> Hi,
> 
> How about it just sorts it all of the time given that the order that svn returns the files really
> does not have any meaning.
> 
> Thanks
> Jason
> 
> --- David Sitsky <sits@...> wrote:
> 
>> Hi Vlad,
>>
>>> I'm creating topics directly from Subversion. One problem I've noticed
>>> immediately
>>> is that file list in a topic is not sorted by filename. Instead, files
>>> are listed in some
>>> semi-sorted order defined by Subversion. I couldn't find any
>>> configuration options
>>> that would trigger file sorting. Did I miss something?
>> The order the files are presented is the same as that in the text 
>> uploaded into Codestriker, or the order obtained from the SCM, in your 
>> case, Subversion.  So no, you didn't miss anything.
>>
>>> Patching the code to sort by filename was rather trivial: in
>>> lib\Codestriker\FileParser\Parser.pm,
>>> insert just before the first return:
>>>
>>>     # Sort the diff chunks by filename, then old linenumber
>>>     @diffs = sort { $a->{filename} cmp $b->{filename} ||
>>> $a->{old_linenumber} <=> $b->{old_linenumber} } @diffs;
>>>  
>>> Would it make sense to make this a configurable feature?
>> I can imagine this could be useful for large reviews.  Rather than 
>> making this a configurable feature (codestriker.conf is also quite large 
>> and complex), it might be useful to have a link instead on the view 
>> topic page above the list of files, for example:
>>
>> Download topic text | ... | Tab width ... | Sort by filename
>>
>> Clicking on the sort by filename link will set a CGI parameter, which 
>> will then execute code similar to what you have, but this would be in 
>> ViewTopic.pm over the list of delta objects.  That way, a reviewer can 
>> choose at runtime which way they'd prefer to see the list of files 
>> presented.
>>
>> What do you think?
>>
>> Cheers,
>> David
>>
>>
>>
>> -------------------------------------------------------
>> All the advantages of Linux Managed Hosting--Without the Cost and Risk!
>> Fully trained technicians. The highest number of Red Hat certifications in
>> the hosting industry. Fanatical Support. Click to learn more
>> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
>> _______________________________________________
>> Codestriker-user mailing list
>> Codestriker-user@...
>> https://lists.sourceforge.net/lists/listinfo/codestriker-user
>>
> 
> 
> __________________________________________________
> Do You Yahoo!?
> Tired of spam?  Yahoo! Mail has the best spam protection around 
> http://mail.yahoo.com 
> 
> 
> -------------------------------------------------------
> All the advantages of Linux Managed Hosting--Without the Cost and Risk!
> Fully trained technicians. The highest number of Red Hat certifications in
> the hosting industry. Fanatical Support. Click to learn more
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
> _______________________________________________
> Codestriker-user mailing list
> Codestriker-user@...
> https://lists.sourceforge.net/lists/listinfo/codestriker-user
>

Hi Hertz,

Thanks for your email.  Yes, one of the last issues to handle is the
actual topic text itself, when it is not ISO-8859-1.  I am working on
these issues now, this also explains why adding the comment is not
working, as the email contains non ISO-8859-1 text when displaying the
source code lines.

I'm working on these issues next.

Just out of interest, what character encoding are your source code files =
in?

Cheers,
David

Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
> Hi David,
>=20
> =20
>=20
> I tried Unicode version and found it's great ^_^, I can now input=20
> Chinese characters in the topics and comments, it's very convenient for=
=20
> us. And I also found the following issues in the Unicode version,
>=20
> =20
>=20
> 1. the comments in the code doesn't support Chinese, they are replaced=20
> by question marks. =A1=B0?=A1=B1
>=20
> =20
>=20
> 2. email notification doesn't work when adding a comment, but it works=20
> fine when creating a topic.
>=20
> =20
>=20
> It always stops here. When I run it in the server, it prompt the object=
=20
> is null, the code is in the line 412 in codestrikerhtml\codestriker.cs=20
> file, the response is null.
>=20
> =20
>=20
> Thank you and best regards,
>=20
> =20
>=20
> Hertz
>=20
> =20
>=20
> =20
>=20

Hi David,

In China, we use GB2312 to encode text(simplified Chinese), for =
Traditional
Chinese, I think it's BIG5, I'll check it again and tell you later.

Hertz

-----Original Message-----
From: codestriker-user-bounces@...
[mailto:codestriker-user-bounces@...] On Behalf Of =
David
Sitsky
Sent: 2006=C4=EA5=D4=C230=C8=D5 7:23
To: Hertz Wang =CD=F5=BB=B3=D6=BE
Cc: codestriker-user@...
Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version

Hi Hertz,

Thanks for your email.  Yes, one of the last issues to handle is the =
actual
topic text itself, when it is not ISO-8859-1.  I am working on these =
issues
now, this also explains why adding the comment is not working, as the =
email
contains non ISO-8859-1 text when displaying the source code lines.

I'm working on these issues next.

Just out of interest, what character encoding are your source code files =
in?

Cheers,
David

Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
> Hi David,
>=20
> =20
>=20
> I tried Unicode version and found it's great ^_^, I can now input=20
> Chinese characters in the topics and comments, it's very convenient=20
> for us. And I also found the following issues in the Unicode version,
>=20
> =20
>=20
> 1. the comments in the code doesn't support Chinese, they are replaced =

> by question marks. =A1=B0?=A1=B1
>=20
> =20
>=20
> 2. email notification doesn't work when adding a comment, but it works =

> fine when creating a topic.
>=20
> =20
>=20
> It always stops here. When I run it in the server, it prompt the=20
> object is null, the code is in the line 412 in=20
> codestrikerhtml\codestriker.cs file, the response is null.
>=20
> =20
>=20
> Thank you and best regards,
>=20
> =20
>=20
> Hertz
>=20
> =20
>=20
> =20
>=20

Hi Hertz,

Try downloading codestriker-1.9.2-alpha-7 now.  There is a new
configuration variable $topic_text_encoding (which defaults to utf8 if
it is not set), which in your case, you should set to 'gb2312'.  Once I
did that, the source files you sent me were rendered correctly, and the
emailed comments were also rendered right.

Please download it and let me know how you go.

Cheers,
David

Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
> Hi David,
>=20
> In China, we use GB2312 to encode text(simplified Chinese), for Traditi=
onal
> Chinese, I think it's BIG5, I'll check it again and tell you later.
>=20
> Hertz
>=20
> -----Original Message-----
> From: codestriker-user-bounces@...
> [mailto:codestriker-user-bounces@...] On Behalf Of Da=
vid
> Sitsky
> Sent: 2006=C4=EA5=D4=C230=C8=D5 7:23
> To: Hertz Wang =CD=F5=BB=B3=D6=BE
> Cc: codestriker-user@...
> Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version
>=20
> Hi Hertz,
>=20
> Thanks for your email.  Yes, one of the last issues to handle is the ac=
tual
> topic text itself, when it is not ISO-8859-1.  I am working on these is=
sues
> now, this also explains why adding the comment is not working, as the e=
mail
> contains non ISO-8859-1 text when displaying the source code lines.
>=20
> I'm working on these issues next.
>=20
> Just out of interest, what character encoding are your source code file=
s in?
>=20
> Cheers,
> David
>=20
> Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
>> Hi David,
>>
>> =20
>>
>> I tried Unicode version and found it's great ^_^, I can now input=20
>> Chinese characters in the topics and comments, it's very convenient=20
>> for us. And I also found the following issues in the Unicode version,
>>
>> =20
>>
>> 1. the comments in the code doesn't support Chinese, they are replaced=
=20
>> by question marks. =A1=B0?=A1=B1
>>
>> =20
>>
>> 2. email notification doesn't work when adding a comment, but it works=
=20
>> fine when creating a topic.
>>
>> =20
>>
>> It always stops here. When I run it in the server, it prompt the=20
>> object is null, the code is in the line 412 in=20
>> codestrikerhtml\codestriker.cs file, the response is null.
>>
>> =20
>>
>> Thank you and best regards,
>>
>> =20
>>
>> Hertz
>>
>> =20
>>
>> =20
>>
>=20
>=20
>=20
>=20
>=20

Hi David,

Thank you for your so quick improvement! I downloaded and tried it =
again, it
works fine when I upload topic text, all the Chinese characters are =
rendered
correctly, but it doesn't work when I compare 2 different cvs version, =
all
the Chinese characters are still not right.

Here is my configuration,
Apache 2.0
ActivePerl 5.8.8.817
MySQL 5.0.12
Version control system - CVS
CodeStriker 1.9.2 alpha7(change the $topic_text_encoding to gb2312, =
change
email.pm to support email authorization)

I went through the database and found the content in the document field =
in
the topic table is not correct(for displaying Chinese character), I am =
not
sure it is mysql bug or my setting error. I set the charset for each =
text
field to gb2312, I'll change to some other charset see what happened.

it still doesn't work when email comments(stop receiving response, same =
as
before).=20

Best regards,

Hertz

-----Original Message-----
From: David Sitsky [mailto:sits@...
Sent: 2006=C4=EA5=D4=C230=C8=D5 15:24
To: Hertz Wang =CD=F5=BB=B3=D6=BE
Cc: codestriker-user@...
Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version

Hi Hertz,

Try downloading codestriker-1.9.2-alpha-7 now.  There is a new =
configuration
variable $topic_text_encoding (which defaults to utf8 if it is not set),
which in your case, you should set to 'gb2312'.  Once I did that, the =
source
files you sent me were rendered correctly, and the emailed comments were
also rendered right.

Please download it and let me know how you go.

Cheers,
David

Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
> Hi David,
>=20
> In China, we use GB2312 to encode text(simplified Chinese), for=20
> Traditional Chinese, I think it's BIG5, I'll check it again and tell =
you
later.
>=20
> Hertz
>=20
> -----Original Message-----
> From: codestriker-user-bounces@...
> [mailto:codestriker-user-bounces@...] On Behalf Of=20
> David Sitsky
> Sent: 2006=C4=EA5=D4=C230=C8=D5 7:23
> To: Hertz Wang =CD=F5=BB=B3=D6=BE
> Cc: codestriker-user@...
> Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version
>=20
> Hi Hertz,
>=20
> Thanks for your email.  Yes, one of the last issues to handle is the=20
> actual topic text itself, when it is not ISO-8859-1.  I am working on=20
> these issues now, this also explains why adding the comment is not=20
> working, as the email contains non ISO-8859-1 text when displaying the
source code lines.
>=20
> I'm working on these issues next.
>=20
> Just out of interest, what character encoding are your source code =
files
in?
>=20
> Cheers,
> David
>=20
> Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
>> Hi David,
>>
>> =20
>>
>> I tried Unicode version and found it's great ^_^, I can now input=20
>> Chinese characters in the topics and comments, it's very convenient=20
>> for us. And I also found the following issues in the Unicode version,
>>
>> =20
>>
>> 1. the comments in the code doesn't support Chinese, they are=20
>> replaced by question marks. =A1=B0?=A1=B1
>>
>> =20
>>
>> 2. email notification doesn't work when adding a comment, but it=20
>> works fine when creating a topic.
>>
>> =20
>>
>> It always stops here. When I run it in the server, it prompt the=20
>> object is null, the code is in the line 412 in=20
>> codestrikerhtml\codestriker.cs file, the response is null.
>>
>> =20
>>
>> Thank you and best regards,
>>
>> =20
>>
>> Hertz
>>
>> =20
>>
>> =20
>>
>=20
>=20
>=20
>=20
>=20

I need to still update the user documentation on the creation of
databases.  For mysql, your create database statement needs to be
something like:

CREATE DATABASE codestrikerdb CHARACTER SET utf8

All the text fields in the database should be stored as utf8 text, not
gb2312, otherwise we will run into problems.

I'd suggest deleting your database, and creating it again with the above
setting, and try again.

I will be away for a week from tomorrow, but back again next Wednesday.

Cheers,
David

Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
> Hi David,
>=20
> Thank you for your so quick improvement! I downloaded and tried it agai=
n, it
> works fine when I upload topic text, all the Chinese characters are ren=
dered
> correctly, but it doesn't work when I compare 2 different cvs version, =
all
> the Chinese characters are still not right.
>=20
> Here is my configuration,
> Apache 2.0
> ActivePerl 5.8.8.817
> MySQL 5.0.12
> Version control system - CVS
> CodeStriker 1.9.2 alpha7(change the $topic_text_encoding to gb2312, cha=
nge
> email.pm to support email authorization)
>=20
> I went through the database and found the content in the document field=
 in
> the topic table is not correct(for displaying Chinese character), I am =
not
> sure it is mysql bug or my setting error. I set the charset for each te=
xt
> field to gb2312, I'll change to some other charset see what happened.
>=20
> it still doesn't work when email comments(stop receiving response, same=
 as
> before).=20
>=20
> Best regards,
>=20
> Hertz
>=20
> -----Original Message-----
> From: David Sitsky [mailto:sits@...
> Sent: 2006=C4=EA5=D4=C230=C8=D5 15:24
> To: Hertz Wang =CD=F5=BB=B3=D6=BE
> Cc: codestriker-user@...
> Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version
>=20
> Hi Hertz,
>=20
> Try downloading codestriker-1.9.2-alpha-7 now.  There is a new configur=
ation
> variable $topic_text_encoding (which defaults to utf8 if it is not set)=
,
> which in your case, you should set to 'gb2312'.  Once I did that, the s=
ource
> files you sent me were rendered correctly, and the emailed comments wer=
e
> also rendered right.
>=20
> Please download it and let me know how you go.
>=20
> Cheers,
> David
>=20
> Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
>> Hi David,
>>
>> In China, we use GB2312 to encode text(simplified Chinese), for=20
>> Traditional Chinese, I think it's BIG5, I'll check it again and tell y=
ou
> later.
>> Hertz
>>
>> -----Original Message-----
>> From: codestriker-user-bounces@...
>> [mailto:codestriker-user-bounces@...] On Behalf Of=20
>> David Sitsky
>> Sent: 2006=C4=EA5=D4=C230=C8=D5 7:23
>> To: Hertz Wang =CD=F5=BB=B3=D6=BE
>> Cc: codestriker-user@...
>> Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version
>>
>> Hi Hertz,
>>
>> Thanks for your email.  Yes, one of the last issues to handle is the=20
>> actual topic text itself, when it is not ISO-8859-1.  I am working on=20
>> these issues now, this also explains why adding the comment is not=20
>> working, as the email contains non ISO-8859-1 text when displaying the
> source code lines.
>> I'm working on these issues next.
>>
>> Just out of interest, what character encoding are your source code fil=
es
> in?
>> Cheers,
>> David
>>
>> Hertz Wang =CD=F5=BB=B3=D6=BE wrote:
>>> Hi David,
>>>
>>> =20
>>>
>>> I tried Unicode version and found it's great ^_^, I can now input=20
>>> Chinese characters in the topics and comments, it's very convenient=20
>>> for us. And I also found the following issues in the Unicode version,
>>>
>>> =20
>>>
>>> 1. the comments in the code doesn't support Chinese, they are=20
>>> replaced by question marks. =A1=B0?=A1=B1
>>>
>>> =20
>>>
>>> 2. email notification doesn't work when adding a comment, but it=20
>>> works fine when creating a topic.
>>>
>>> =20
>>>
>>> It always stops here. When I run it in the server, it prompt the=20
>>> object is null, the code is in the line 412 in=20
>>> codestrikerhtml\codestriker.cs file, the response is null.
>>>
>>> =20
>>>
>>> Thank you and best regards,
>>>
>>> =20
>>>
>>> Hertz
>>>
>>> =20
>>>
>>> =20
>>>
>>
>>
>>
>>
>=20
>=20
>=20
>=20
>=20
> -----------------------------------------------------------------------=
-
>=20
> _______________________________________________
> Codestriker-user mailing list
> Codestriker-user@...
> https://lists.sourceforge.net/lists/listinfo/codestriker-user

> Thank you for your so quick improvement! I downloaded and tried it again, it
> works fine when I upload topic text, all the Chinese characters are rendered
> correctly, but it doesn't work when I compare 2 different cvs version, all
> the Chinese characters are still not right.

This is an interesting question actually - when you do a CVS diff, what
encoding is the text output as?  Is it gb2312 or is it utf8?  Perhaps if
you can email me an example diff, that would be helpful.

Cheers,
David

Hi David

Some information for you,

I changed the :utf8 to :gb2312 in SubmitNewTopic.pm file(line 191, 192)

191  	binmode $temp_topic_fh, ':gb2312';
192  	binmode $temp_error_fh, ':gb2312';

Now the cvs diff can be displayed correctly.

it's a hard-coded solution, maybe you can think how to change this in =
your
code.

Ps. Adding Chinese comments are still not supported, always stop on
"receiving response..." step, and the comments cannot be put into the
database.

Hertz


-----Original Message-----
From: David Sitsky [mailto:sits@...
Sent: 2006=C4=EA5=D4=C231=C8=D5 7:22
To: Hertz Wang =CD=F5=BB=B3=D6=BE
Cc: codestriker-user@...
Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version

> Thank you for your so quick improvement! I downloaded and tried it=20
> again, it works fine when I upload topic text, all the Chinese=20
> characters are rendered correctly, but it doesn't work when I compare=20
> 2 different cvs version, all the Chinese characters are still not =
right.

This is an interesting question actually - when you do a CVS diff, what
encoding is the text output as?  Is it gb2312 or is it utf8?  Perhaps if =
you
can email me an example diff, that would be helpful.

Cheers,
David

Hi David,

Thank you for you help!

I had changed the codestrikerdb database/table/field charset to utf8, =
ascii
and gb2312, the results were same. The Chinese characters are supported =
when
uploading topic file, but not supported when comparing 2 different
revisions. I checked database content, there is no different when I =
switch
different charset.

I guess there might be something wrong while inserting the comparison =
result
to topic or delta table, it's not database charset problem.

I attached 2 cvs diff result, hope it helpful for you.

Have a nice trip! Hope you can come to China, I will host you with open
arms.=20

Hertz


-----Original Message-----
From: David Sitsky [mailto:sits@...
Sent: 2006=C4=EA5=D4=C231=C8=D5 7:22
To: Hertz Wang =CD=F5=BB=B3=D6=BE
Cc: codestriker-user@...
Subject: Re: [Codestriker-user] Issues in the 1.9.2 unicode version

> Thank you for your so quick improvement! I downloaded and tried it=20
> again, it works fine when I upload topic text, all the Chinese=20
> characters are rendered correctly, but it doesn't work when I compare=20
> 2 different cvs version, all the Chinese characters are still not =
right.

This is an interesting question actually - when you do a CVS diff, what
encoding is the text output as?  Is it gb2312 or is it utf8?  Perhaps if =
you
can email me an example diff, that would be helpful.

Cheers,
David