Allow suppressing message extraction warnings, providing a prefix for generated files, and format w…

Allow suppressing message extraction warnings, providing a prefix for generated files, and format warnings better.

Committed: https://code.google.com/p/dart/source/detail?r=21959

[Alan Knight, 2013-04-23 00:34:09]

This allows suppressing the warnings when extracting messages (and suppresses
them automatically when re-parsing to generate code). It also allows a prefix
for the name of the generated files.

[Siggi Cherem (dart-lang), 2013-04-23 02:12:51]

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
File pkg/intl/test/data_directory.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
pkg/intl/test/data_directory.dart:29: for (var each in components) {
nit: 'each' => 'segment' or 'component'?

(generally, I think a noun describing what each thing is makes the body the loop
easier to understand.)

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
File pkg/intl/test/message_extraction/extract_to_json.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/extract_to_json.dart:33: var targetDir =
findOutputDirectory(args);
why parse it both here and with the output-dir option below?

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/extract_to_json.dart:37: void
setTargetDir(value) { if (value != null) targetDir = value;};
is it possible for it to be null? (given that you provided a default value?

[Emily Fortuna, 2013-04-23 17:24:48]

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
File pkg/intl/test/data_directory.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
pkg/intl/test/data_directory.dart:29: for (var each in components) {
On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> nit: 'each' => 'segment' or 'component'?
> 
> (generally, I think a noun describing what each thing is makes the body the
loop
> easier to understand.)

+1

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
File pkg/intl/test/message_extraction/message_extraction_test.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/message_extraction_test.dart:52: if (s != null
&& s.startsWith("--")) { // Don't touch command-line options.
indentation is off on this method (2 spaces)

[Alan Knight, 2013-04-23 17:59:56]

PTAL

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
File pkg/intl/test/data_directory.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
pkg/intl/test/data_directory.dart:29: for (var each in components) {
On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> nit: 'each' => 'segment' or 'component'?
> 
> (generally, I think a noun describing what each thing is makes the body the
loop
> easier to understand.)

This is moot, because this file's changes were because of a change to takeWhile
that made the previous version fail. That change broke lots of other stuff and
has been reverted, so reverting this file to the initial version.

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
File pkg/intl/test/message_extraction/extract_to_json.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/extract_to_json.dart:33: var targetDir =
findOutputDirectory(args);
On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> why parse it both here and with the output-dir option below?
Removed. See next point.

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/extract_to_json.dart:37: void
setTargetDir(value) { if (value != null) targetDir = value;};
On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> is it possible for it to be null? (given that you provided a default value?

Changed it so that we don't have a default value for the argument and call
findOutputDirectory here if no value is provided.

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
File pkg/intl/test/message_extraction/message_extraction_test.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/message_extraction_test.dart:52: if (s != null
&& s.startsWith("--")) { // Don't touch command-line options.
On 2013/04/23 17:24:48, Emily Fortuna wrote:
> indentation is off on this method (2 spaces)

Done.

[Siggi Cherem (dart-lang), 2013-04-23 18:56:37]

lgtm (with the comments below)

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
File pkg/intl/test/data_directory.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
pkg/intl/test/data_directory.dart:29: for (var each in components) {
On 2013/04/23 17:59:56, Alan Knight wrote:
> On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> > nit: 'each' => 'segment' or 'component'?
> > 
> > (generally, I think a noun describing what each thing is makes the body the
> loop
> > easier to understand.)
> 
> This is moot, because this file's changes were because of a change to
takeWhile
> that made the previous version fail. That change broke lots of other stuff and
> has been reverted, so reverting this file to the initial version.

sure - I noticed that the original code is also using 'each' as a function
argument, can you make the change there instead?

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
File pkg/intl/test/message_extraction/extract_to_json.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/extract_to_json.dart:37: void
setTargetDir(value) { if (value != null) targetDir = value;};
On 2013/04/23 17:59:56, Alan Knight wrote:
> On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> > is it possible for it to be null? (given that you provided a default value?
> 
> Changed it so that we don't have a default value for the argument and call
> findOutputDirectory here if no value is provided. 

I guess part of my point is, if you are now using the ArgsParser, there is no
need to use 'fundOutputDirectory'. IIUC that function was doing a subset of the
parsing that argParser does already. Might be enough to replace the default
value with '.'.

[Alan Knight, 2013-04-23 21:37:19]

PTAL

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
File pkg/intl/test/data_directory.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/data_directory....
pkg/intl/test/data_directory.dart:29: for (var each in components) {
On 2013/04/23 18:56:37, Siggi Cherem (dart-lang) wrote:
> On 2013/04/23 17:59:56, Alan Knight wrote:
> > On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> > > nit: 'each' => 'segment' or 'component'?
> > > 
> > > (generally, I think a noun describing what each thing is makes the body
the
> > loop
> > > easier to understand.)
> > 
> > This is moot, because this file's changes were because of a change to
> takeWhile
> > that made the previous version fail. That change broke lots of other stuff
and
> > has been reverted, so reverting this file to the initial version.
> 
> sure - I noticed that the original code is also using 'each' as a function
> argument, can you make the change there instead?

Done.

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
File pkg/intl/test/message_extraction/extract_to_json.dart (right):

https://codereview.chromium.org/13898014/diff/1/pkg/intl/test/message_extract...
pkg/intl/test/message_extraction/extract_to_json.dart:37: void
setTargetDir(value) { if (value != null) targetDir = value;};
On 2013/04/23 18:56:37, Siggi Cherem (dart-lang) wrote:
> On 2013/04/23 17:59:56, Alan Knight wrote:
> > On 2013/04/23 02:12:51, Siggi Cherem (dart-lang) wrote:
> > > is it possible for it to be null? (given that you provided a default
value?
> > 
> > Changed it so that we don't have a default value for the argument and call
> > findOutputDirectory here if no value is provided. 
> 
> I guess part of my point is, if you are now using the ArgsParser, there is no
> need to use 'fundOutputDirectory'. IIUC that function was doing a subset of
the
> parsing that argParser does already. Might be enough to replace the default
> value with '.'.

Doh! I'd forgotten that was what that was doing. Deleted findOutputDirectory and
its uses.

[Siggi Cherem (dart-lang), 2013-04-24 05:26:20]

lgtm

[Alan Knight, 2013-04-24 17:03:43]

Committed patchset #3 manually as r21959 (presubmit successful).