Change how the Dart library name is generated

Change how the Dart library name is generated

The package specifier from the .proto file is now used to set the
library name. In the case where there is no package specifier the
previous method of using the file name is used except that the first
letter is not changed into upper-case.

R=sigmund@google.com
BUG=https://github.com/dart-lang/dart-protoc-plugin/issues/7

Committed: https://github.com/dart-lang/protoc-plugin/commit/640e8bc

[Siggi Cherem (dart-lang), 2013-09-13 17:06:52]

lgtm, minor nits and suggestions below

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dart
File lib/file_generator.dart (right):

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:32: String get fqname => _fileDescriptor.package == null
Looking at your check below made me wonder, can [package] ever be null? If yes,
maybe line 55 should check for null too? If not is this check needed or should
it also check for ''?

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:55: if (_fileDescriptor.package.length > 0) return
_fileDescriptor.package;
style nit: in our code we normally write this as: (_fileDescriptor.package !=
'')

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:56: return
_fileNameWithoutExtension(protoFilePath).replaceAll('-', '_') + ".pb";
nit: 80 col

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:56: return
_fileNameWithoutExtension(protoFilePath).replaceAll('-', '_') + ".pb";
I'm ok with it, but do we really need 'pb' in the library name?

[Søren Gjesse, 2013-09-16 09:22:03]

Committed patchset #2 manually as r640e8bc (presubmit successful).

[Søren Gjesse, 2013-09-16 09:26:02]

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dart
File lib/file_generator.dart (right):

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:32: String get fqname => _fileDescriptor.package == null
On 2013/09/13 17:06:53, Siggi Cherem (dart-lang) wrote:
> Looking at your check below made me wonder, can [package] ever be null? If
yes,
> maybe line 55 should check for null too? If not is this check needed or should
> it also check for ''? 

The package is an optional string field in a protobuf, and missing  values are
always their default value which for strings is the empty string. Removed the
null check.

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:55: if (_fileDescriptor.package.length > 0) return
_fileDescriptor.package;
On 2013/09/13 17:06:53, Siggi Cherem (dart-lang) wrote:
> style nit: in our code we normally write this as: (_fileDescriptor.package !=
> '')

Done.

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:56: return
_fileNameWithoutExtension(protoFilePath).replaceAll('-', '_') + ".pb";
On 2013/09/13 17:06:53, Siggi Cherem (dart-lang) wrote:
> nit: 80 col

Done.

https://chromiumcodereview.appspot.com/23522040/diff/1/lib/file_generator.dar...
lib/file_generator.dart:56: return
_fileNameWithoutExtension(protoFilePath).replaceAll('-', '_') + ".pb";
On 2013/09/13 17:06:53, Siggi Cherem (dart-lang) wrote:
> I'm ok with it, but do we really need 'pb' in the library name?

I also thought about that when making the change. Removed it.