Re: [emms-help] [PATCH] New metadata extraction program

[Petteri Hintsanen]

On 19/10/15 16:30, Rasmus wrote:

> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -27,6 +27,7 @@ Ye Wenbin              <address@hidden>
> >   Yoni (Johnathan) Rabkin  <address@hidden>
> >   mathias.dahl           <mathias.dahl>
> >   Rasmus Pank Roulund      <address@hidden>
> > +Petteri Hintsanen        <address@hidden>
>
> Maybe make this a separate commit.

OK.

> Would it be better to use old-style loops than making this requirement?
> Personally, I think C++2011 is fine by now.

For-loops are not the only C++11 thing, auto keyword and initializer 
list syntax are there too.  But they can be easily replaced if you want 
to stick to C++98.  It would be a bit uglier.

> The naming is confusing.  emms-print-metadata is also using taglib, though
> not the property list of the C++ binding.  I say we stick to one program.
> The implementation language doesn’t matter to the end user and the
> functionality is the same.

OK.  I think this is up to maintainers to decide.  I'm happy with any 
decision.

> > +int
> > +main (int argc, char* argv[])
>
> Is it on purpose that you write int main in two lines?

The style follows GNU Coding Standards.
http://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting
(Personally I do not fancy the style at all, but it is a matter or 
taste.  Consistency is much more important.)

> > +  TagLib::FileRef file (argv[1]);
> > +  if (file.isNull ()) return 1;
>
> Should an helpful error/message be displayed here?

Yes, the original C-version seems to do that too.

Thanks,
Petteri