[MPlayer-dev-eng] printf -> mp_msg conversion (etc.), first patches

D Richard Felker III dalias at aerifal.cx
Sat May 22 23:01:44 CEST 2004


On Sat, May 22, 2004 at 03:36:07PM -0400, The Wanderer wrote:
> >>The third patch removes all functional printfs from mencoder.c.
> >
> >Agree, except IMO you shouldn't change fprintf(stderr, ...) to
> >MSGL_ERROR. Just use MSGL_FIXME like everything else.
> 
> My reasoning was that changing it like that loses information - the
> original was targeted at stderr, and just using FIXME for it means the
> prospective fixer can't tell something which was fprintf(stderr, ...)
> from something which was printf(...). If you think it's important 
> enough, however, I can go back and correct them.

IMO the original was targetted at stderr because the person who wrote
those lines was competent and realized that ALL messages should go to
stderr, so stdout could (hypothetically in the future) be used as the
output file. Once you fix it to everything goes thru mp_msg, we can
make an option to send all messages to stderr, and then this will be a
reality.

In any case, the messages in question are not errors, so they
shouldn't be MSGL_ERROR.

> Should I separate the printf -> mp_msg changes and the string moves into
> different patches, in the future? Doing so would slow me down, since I
> would have to go through files twice (and, unless I've missed something,
> wait for the first to be committed before beginning work on the second),
> but it might be worth it.

IMO yes, they should be separate. It's not as hard as you think to do
it in 2 passes. Just look at the diff file for a list of locations you
need to go back to. Also you can just diff your two edits directly,
rather than using cvs diff, so you don't have to wait to make the
second patch set.

Rich




More information about the MPlayer-dev-eng mailing list