Re: [webkit-dev] Using namespace std

2012-05-16 Thread Allan Sandfeld Jensen
On Tuesday 15 May 2012, Darin Adler wrote:
 On May 15, 2012, at 5:48 AM, Allan Sandfeld Jensen wrote:
  To me it seems like an odd practice, so I would like to ask what original
  rationale behind that style guideline is
 
 Adding a list of using declarations like using std::min to the top of
 each source file would give us another ongoing maintenance job. The list
 in each file would grow, we’d not remember to remove them when they are no
 longer needed, and so on.
 
 Adding using namespace std to the top of a source file deals with 99% of
 the issue without the trouble of maintaining another list at the top of
 each file.
 
That is reasonable. I am just generally wary of opening external namespaces, 
since they can eventually expand and end up causing conflicts.

 When there is a conflict, there are typically many simple ways to resolve
 the conflict. Removing using namespace std entirely is not the only
 solution and we should avoid it if possible. I see in your patch in
 https://bugs.webkit.org/show_bug.cgi?id=86465 that you have decided to
 remove it from many files, and before we do that I suggest we first
 investigate the other solutions.
 
 Unfortunately your patch does not say what the conflict is; I can’t make a
 helpful suggestion for an alternate solution without that information.
 
The conflict is between isinf, isnan and std::isinf and std::isnan, but the 
conflict only exists in C++11 when constexpr versions are introduced.

Since the problem exists in both GCC 4.6 and 4.7, I see no alternative than 
ensuring any files that uses isinf or isnan does not open the std namespace. 
This is only around 10 files in total as you can see in my patch.

Also as I note in  https://bugs.webkit.org/show_bug.cgi?id=59249, the only std 
functions used in all these cases are std::min, std::max and 
std::numeric_limits. If these three functions were declared using in 
MathExtra.h then none of these files would need any using std declarations.

Best regards
`Allan

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Using namespace std

2012-05-16 Thread Allan Sandfeld Jensen
On Tuesday 15 May 2012, Darin Adler wrote:
 On May 15, 2012, at 10:04 AM, Allan Sandfeld Jensen wrote:
  The conflict is between isinf, isnan and std::isinf and std::isnan, but
  the conflict only exists in C++11 when constexpr versions are
  introduced.
 
 We should try harder to come up with a solution in MathExtras.h. Even one
 that uses macros.
 
 If I understand correctly, the conflict here is not between some WebKit
 namespace and the std namespace, it’s in the C++ library itself between
 the global namespace and the std namespace. I think that’s a bug in the
 C++ library, and MathExtras.h is the perfect place for a workaround for
 that bug.
 
True. If we can find a method that would work, that would be the optimal 
solution, but it is a difficult case to work around. It is not that I haven't 
tried, but modifying those ~10 files was just a lot easier in the end.

  Since the problem exists in both GCC 4.6 and 4.7, I see no alternative
  than ensuring any files that uses isinf or isnan does not open the std
  namespace. This is only around 10 files in total as you can see in my
  patch.
 
 Sure, but that assumes we won’t use more things from the std namespace in
 the future, and I am not sure that’s a good assumption.
 
You don't have to open the namespace to use the methods within. I prefer to 
use the 'std::' prefix. I only kept them in this dcase to keep the changes to 
a minimum.

Best regards
`Allan


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] paged media update?

2012-05-16 Thread Milian Wolff
On Tuesday 15 May 2012 15:20:14 adam wrote:
 hi

Hey there!

 Just wanted to know if Milian and Dave Hyatt spoke about the paged media
 updates? In the next months I might also be able to find some resources
 to help but just wanting to know what the current plan is...

I'm still waiting for feedback from Dave Hyatt, yet I fear he's still on sick 
leave. Lets hope he gets well soon, I'll keep you posted.

PS: Great to hear you are interested in working on this as well - I'd 
appreciate any kind of help in getting this beast implemented!

-- 
Milian Wolff | milian.wo...@kdab.com | Software Engineer
KDAB (Deutschland) GmbHCo KG, a KDAB Group company
Tel. Germany +49-30-521325470, Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-independent software solutions


signature.asc
Description: This is a digitally signed message part.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] paged media update?

2012-05-16 Thread adam

Ah, Sorry I didnt know he was sick. Best wishes to him.


adam

On 16/05/12 11:56, Milian Wolff wrote:

On Tuesday 15 May 2012 15:20:14 adam wrote:

hi


Hey there!


Just wanted to know if Milian and Dave Hyatt spoke about the paged media
updates? In the next months I might also be able to find some resources
to help but just wanting to know what the current plan is...


I'm still waiting for feedback from Dave Hyatt, yet I fear he's still on sick
leave. Lets hope he gets well soon, I'll keep you posted.

PS: Great to hear you are interested in working on this as well - I'd
appreciate any kind of help in getting this beast implemented!




___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Using namespace std

2012-05-16 Thread Allan Sandfeld Jensen
On Wednesday 16 May 2012, Allan Sandfeld Jensen wrote:
 On Tuesday 15 May 2012, Darin Adler wrote:
  On May 15, 2012, at 10:04 AM, Allan Sandfeld Jensen wrote:
   The conflict is between isinf, isnan and std::isinf and std::isnan, but
   the conflict only exists in C++11 when constexpr versions are
   introduced.
  
  We should try harder to come up with a solution in MathExtras.h. Even one
  that uses macros.
  
  If I understand correctly, the conflict here is not between some WebKit
  namespace and the std namespace, it’s in the C++ library itself between
  the global namespace and the std namespace. I think that’s a bug in the
  C++ library, and MathExtras.h is the perfect place for a workaround for
  that bug.
 
 True. If we can find a method that would work, that would be the optimal
 solution, but it is a difficult case to work around. It is not that I
 haven't tried, but modifying those ~10 files was just a lot easier in the
 end.
 
Okay I found a solution and have updated the patch in 
https://bugs.webkit.org/show_bug.cgi?id=86465. It is not pretty, but it works 
 
I should point out though, that there is another conflict which is entirely 
our own fault. It is between WTF::bind and the new std::bind from C++11, we 
have even declared 'using WTF::bind' in the global namespace, which makes 
conflicts much easier to trigger. On top of that several of the files using 
bind, had declared 'using namespace std' even though they do not using any 
functions from std.

We can work around issues like this, but I would still prefer if we advised 
against 'using namespace std', as seen even more clearly in the 'bind' case, 
it can easily cause problems when upgrading standard libraries or compilers.

Best regards.
`Allan Jensen
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Using namespace std

2012-05-16 Thread Darin Adler
On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote:

 there is another conflict which is entirely our own fault. It is between 
 WTF::bind and the new std::bind from C++11

We should find a good solution for this. I’d suggest talking with Anders 
Carlsson about it.

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Using namespace std

2012-05-16 Thread Anders Carlsson

On May 16, 2012, at 9:31 AM, Darin Adler da...@apple.com wrote:

 On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote:
 
 there is another conflict which is entirely our own fault. It is between 
 WTF::bind and the new std::bind from C++11
 
 We should find a good solution for this. I’d suggest talking with Anders 
 Carlsson about it.

I've run into this and had to use the fully qualified WTF::bind in those cases.

FWIW, I don't think we really need using directives for the std namespace - the 
fully qualified name is short enough and I like the additional clarity that 
we're calling something in the standard library.

- Anders
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] paged media update?

2012-05-16 Thread David Hyatt
The difficulty with implementing paged media right now is that I'm beginning 
work on re-architecting pagination to unify columns, pages and regions. You can 
see the beginnings of this with the new multi-column classes that have been 
added to the tree.

The basic idea is that multi-column layout is being separated from RenderBlock 
and moved to a subclass, RenderMultiColumnBlock. Similar to how regions work, 
the content is represented by a flow thread and by regions that the flow thread 
is placed into. In order to avoid creating thousands of regions for columns, 
I've introduced the concept of a region set, i.e., RenderMultiColumnSet.

Pages ultimately need to be rebuilt to work the same way, i.e., to have a 
RenderPageFlowThread and a RenderPageSet. Once we have actual renderers that 
represent these objects, it will be a lot easier to make real boxes for things 
like margins, headers, footers, etc. The idea is that a set represents a 
contiguous run of objects that can all be rendered identically. If you're 
forced to have a different kind of object, e.g., a page with a different 
width/height or different style, then you break up the RenderPageSet and have 
multiple sets for each (this is similar to how regions work by default, 
although I hope to apply the same optimization to regions eventually as well if 
runs of same width/height regions are detected that are also siblings in the 
render tree).

dave

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Using namespace std

2012-05-16 Thread Jarred Nicholls
On Wed, May 16, 2012 at 3:04 PM, Anders Carlsson ander...@apple.com wrote:


 On May 16, 2012, at 9:31 AM, Darin Adler da...@apple.com wrote:

  On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote:
 
  there is another conflict which is entirely our own fault. It is
 between WTF::bind and the new std::bind from C++11
 
  We should find a good solution for this. I’d suggest talking with Anders
 Carlsson about it.

 I've run into this and had to use the fully qualified WTF::bind in those
 cases.

 FWIW, I don't think we really need using directives for the std namespace
 - the fully qualified name is short enough and I like the additional
 clarity that we're calling something in the standard library.


Ditto.  After awhile, one gets used to it and I for one especially
appreciate the additional clarity that it brings when reading code in a
very large project.



 - Anders
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Using namespace std

2012-05-16 Thread James Robinson
On Wed, May 16, 2012 at 12:04 PM, Anders Carlsson ander...@apple.comwrote:


 On May 16, 2012, at 9:31 AM, Darin Adler da...@apple.com wrote:

  On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote:
 
  there is another conflict which is entirely our own fault. It is
 between WTF::bind and the new std::bind from C++11
 
  We should find a good solution for this. I’d suggest talking with Anders
 Carlsson about it.

 I've run into this and had to use the fully qualified WTF::bind in those
 cases.

 FWIW, I don't think we really need using directives for the std namespace
 - the fully qualified name is short enough and I like the additional
 clarity that we're calling something in the standard library.


I would be strongly in favor of using fully qualified names for std classes
(std::bind instead of just bind).  This isn't a problem in other large
codebases, even ones that use far more types from std:: (like containers)
and that have column limits.

- James


 - Anders
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Using namespace std

2012-05-16 Thread Darin Adler
On May 16, 2012, at 12:47 PM, James Robinson wrote:

 On Wed, May 16, 2012 at 12:04 PM, Anders Carlsson ander...@apple.com wrote:
 
 FWIW, I don't think we really need using directives for the std namespace - 
 the fully qualified name is short enough and I like the additional clarity 
 that we're calling something in the standard library.
 
 I would be strongly in favor of using fully qualified names for std classes 
 (std::bind instead of just bind).

I’m OK with that.

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Ryosuke Niwa
Hi,

There has been some complaints / discussions about how syntax in
test_expectations.txt is confusing (and I agree with you) on webkit-dev and
at contributors' meeting.

So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to
simplify syntax in test_expectation.txt as follows:

   - Bug modififers are bug numebr themselves for WebKit and URLs for
   non-WebKit bugs
   e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and
BUGCR12345respectively
   - : and = delimiters are no longer needed
   - All modifiers and expectations show before test name
   e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC :
   test.html = TEXT IMAGE


*Note*:
Just listing a test DOES NOT automatically skip a test even after my patch
is applied:
test.html
will result in a warning, and we still expect it to pass. I'll cite Ojan's
comment for the rationale:

 The bigger problem with implying SKIP, is how do you decide when SKIP
 isn't applied? That needs some hidden set of rules that's too complicated
 IMO.


We can come back to this problem later if we wanted but let us start from
the above cleanups first.

For those who are interested, we're also going to rename
test_expectations.txt to something more WebKitty on
https://bugs.webkit.org/show_bug.cgi?id=86690.

Best regards,
Ryosuke Niwa
Software Engineer
Google Inc.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Benjamin Poulain
On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote:
 e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC :
 test.html = TEXT IMAGE

I wonder how this is gonna help making test_expectation.txt less
obscure. If anything, I would make the file more verbose.

I also think it could be split between regular failure, test
expectations that we expect to diverge, and temporary failures that
just need a rebaseline.
For temporary failure, a system that would avoid conflicts on
update/pull would be nice.

Cheers,
Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Ryosuke Niwa
Any concrete proposals?
On May 16, 2012 9:23 PM, Benjamin Poulain benja...@webkit.org wrote:

 On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote:
  e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC :
  test.html = TEXT IMAGE

 I wonder how this is gonna help making test_expectation.txt less
 obscure. If anything, I would make the file more verbose.

 I also think it could be split between regular failure, test
 expectations that we expect to diverge, and temporary failures that
 just need a rebaseline.
 For temporary failure, a system that would avoid conflicts on
 update/pull would be nice.

 Cheers,
 Benjamin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Dirk Pranke
On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote:
 Hi,

 There has been some complaints / discussions about how syntax in
 test_expectations.txt is confusing (and I agree with you) on webkit-dev and
 at contributors' meeting.

 So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to
 simplify syntax in test_expectation.txt as follows:

 Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit
 bugs
 e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345
 respectively
 : and = delimiters are no longer needed
 All modifiers and expectations show before test name
 e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC :
 test.html = TEXT IMAGE


As I've commented in the bug, I'm not a fan of the proposed syntax above :(


 Note:
 Just listing a test DOES NOT automatically skip a test even after my patch
 is applied:
 test.html
 will result in a warning, and we still expect it to pass. I'll cite Ojan's
 comment for the rationale:

 The bigger problem with implying SKIP, is how do you decide when SKIP
 isn't applied? That needs some hidden set of rules that's too complicated
 IMO.


As I've also noted in the bug, I don't follow this ... how is this any
more complicated than if there's no modifier, default to SKIP? Why
is defaulting to PASS any easier?


 We can come back to this problem later if we wanted but let us start from
 the above cleanups first.

 For those who are interested, we're also going to rename
 test_expectations.txt to something more WebKitty
 on https://bugs.webkit.org/show_bug.cgi?id=86690.


I would have preferred it if you'd phrased this as proposed renaming
test_expectations.txt :). It's not clear to me that renaming it to
any of the other suggestions thus far are improvements, and there are
lots of people (and scripts) used to the existing names.

I'm sorry for sounding negative, but a lot of this feels like
superficial stylistic tweaks, so I would be inclined to leave things
the way they are unless there is a clear consensus that people prefer
the new syntax?

-- Dirk
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Maciej Stachowiak

On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote:

 Hi,
 
 There has been some complaints / discussions about how syntax in 
 test_expectations.txt is confusing (and I agree with you) on webkit-dev and 
 at contributors' meeting.
 
 So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to 
 simplify syntax in test_expectation.txt as follows:
 Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit 
 bugs
 e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 
 respectively
I think I might prefer webkit.org/b/12345 - it's a bit less concise but the 
meaning is more contextually obvious and you can cut and paste it into a 
browser address field to follow the link, rather than having to know what site 
to enter it into.
 : and = delimiters are no longer needed
 All modifiers and expectations show before test name
 e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : 
 test.html = TEXT IMAGE
Would it be possible to also change the modifiers to lower case or mixed case? 
All-caps is hard to type and uncomfortable to read. As for where the modifiers 
go, was there a logical basis for the previous separation into two groups? 
Would we be losing something by doing that?

 
Cheers,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Ryosuke Niwa
On Wed, May 16, 2012 at 9:59 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote:
  For those who are interested, we're also going to rename
  test_expectations.txt to something more WebKitty
  on https://bugs.webkit.org/show_bug.cgi?id=86690.

 I would have preferred it if you'd phrased this as proposed renaming
 test_expectations.txt :). It's not clear to me that renaming it to
 any of the other suggestions thus far are improvements, and there are
 lots of people (and scripts) used to the existing names.


It needs to be renamed to be consistent with the naming convention we use
in LayoutTests and the rest of the WebKit directories (e.g. ChangeLog,
Skipped, etc...). Besides files in webkitpy, few tests, and maybe few other
files, we don't use underscore delimited file names. I even think that we
shouldn't use it in webkitpy either but that's an orthogonal issue.

I'm signing up to do the work since I've been always annoyed and nobody has
signed up to do it so far.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Benjamin Poulain
On Wed, May 16, 2012 at 9:34 PM, Ryosuke Niwa rn...@webkit.org wrote:
 Any concrete proposals?

I would go for something verbose in a easy to understand format. E.g.
{ test: 'fast/html/keygen.html',
  platform: 'mac, linux',
  bug: 'webkit.org/b/6',
  expectedResults: ['crash', 'text', 'image'],
  action: 'skip' }

For the tests that need rebaseline or new results, I would leave them
out of the file.
I would use an second file, with entries sorted by bug number.

I don't know all the problems related to test_expectations so that is
just some ideas from what I encountered in the past.

Cheers,
Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Dirk Pranke
On Wed, May 16, 2012 at 10:30 PM, Maciej Stachowiak m...@apple.com wrote:

 On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote:

 Hi,

 There has been some complaints / discussions about how syntax in
 test_expectations.txt is confusing (and I agree with you) on webkit-dev and
 at contributors' meeting.

 So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to
 simplify syntax in test_expectation.txt as follows:

 Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit
 bugs
 e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345
 respectively

 I think I might prefer webkit.org/b/12345 - it's a bit less concise but the
 meaning is more contextually obvious and you can cut and paste it into a
 browser address field to follow the link, rather than having to know what
 site to enter it into.

 : and = delimiters are no longer needed
 All modifiers and expectations show before test name
 e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC :
 test.html = TEXT IMAGE

 Would it be possible to also change the modifiers to lower case or mixed
 case? All-caps is hard to type and uncomfortable to read. As for where the
 modifiers go, was there a logical basis for the previous separation into two
 groups? Would we be losing something by doing that?


There was a semi-logical basis, in that the stuff on the right of the
test clearly specified the outcome of the test (PASS, IMAGE, TEXT,
etc.). The stuff on the left was less well defined: there's the bug
numbers, the platform/configuration info (MAC LINUX RELEASE, etc.),
and some other stuff that there is less of a good place for (SLOW,
SKIP, WONTFIX).

I think it makes sense to syntactically separate at least the
platform/configuration keywords from the outcome keywords. It might be
nice to separate the other things somehow as well, but I don't have
any great ideas for things that are clearly better than the existing
left/right convention.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-16 Thread Ryosuke Niwa
On Wed, May 16, 2012 at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Wed, May 16, 2012 at 10:30 PM, Maciej Stachowiak m...@apple.com wrote:
  On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote:
  There has been some complaints / discussions about how syntax in
  test_expectations.txt is confusing (and I agree with you) on webkit-dev
 and
  at contributors' meeting.
 
  So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to
  simplify syntax in test_expectation.txt as follows:
 
  Bug modififers are bug numebr themselves for WebKit and URLs for
 non-WebKit
  bugs
  e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345
  respectively
 
  I think I might prefer webkit.org/b/12345 - it's a bit less concise but
 the
  meaning is more contextually obvious and you can cut and paste it into a
  browser address field to follow the link, rather than having to know what
  site to enter it into.
 
  : and = delimiters are no longer needed
  All modifiers and expectations show before test name
  e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC :
  test.html = TEXT IMAGE
 
  Would it be possible to also change the modifiers to lower case or mixed
  case? All-caps is hard to type and uncomfortable to read. As for where
 the
  modifiers go, was there a logical basis for the previous separation into
 two
  groups? Would we be losing something by doing that?
 

 There was a semi-logical basis, in that the stuff on the right of the
 test clearly specified the outcome of the test (PASS, IMAGE, TEXT,
 etc.). The stuff on the left was less well defined: there's the bug
 numbers, the platform/configuration info (MAC LINUX RELEASE, etc.),
 and some other stuff that there is less of a good place for (SLOW,
 SKIP, WONTFIX).


What I find most annoying with the current syntax is that modifiers and
expectations appear on the opposite ends of a test name. So if I'm copying
 pasting a test name to add a new entry to test_expectations.txt, then I
would have to type something on both ends instead of typing stuff in before
or after pasting the test name.

Taking Benjamin's idea to make the meaning of tokens explicit and Maciej's
(and my original) proposal to use webkit.org/b/12345, how about something
like
webkit.org/12356 [Win Mac] [Text] failing-test.html
rniwa [Linux] [Skip] temporarily-skipped-test.html
Here, platforms and expectations are wrapped around [] and non-URL comments
are wrapped in . (I think we should just use IRC nick).

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev