Re: [flashrom] [PATCH 2/2] add printing of chip voltage ranges to print_wiki.c

[Uwe Hermann]

On Mon, Jun 27, 2011 at 06:30:14AM +0200, Stefan Tauner wrote:
> static const char wiki_header[] = "= Supported devices =\n\n\
>
>  <div style=\"margin-top:0.5em; padding:0.5em 0.5em 0.5em 0.5em; \
> -background-color:#eeeeee; align:right; border:1px solid #aabbcc;\"><small>\n\
> +background-color:#eeeeee; text-align:right; border:1px solid #aabbcc;\">\
> +<small>\n\

Correct, but not sure if this was supposed to be right-aligned anyway.
The output looks different with the patch, but it's nice too :)
> -static const char chip_th[] = "{| border="0" style="font-size: smaller" \
> -valign="top"\n|- bgcolor="#6699dd"\n! align="left" | Vendor\n\
> -! align="left" | Device\n! align="left" | Size / kB\n\
> -! align="left" | Type\n! align="left" colspan="4" | Status\n\n\
> +static const char chip_th[] = "{\
> +| border="0" style="font-size: smaller"valign="top"\n\

^
                    Missing space, this screws up the output.
> +|- bgcolor="#6699dd"\n\
> +! align="left" | Vendor\n\
> +! align="left" | Device\n\
> +! align="center" | Size [kB]\n\

^
     Please replace the space with "&nbsp;" here.
> +! align="center" | Type\n\
> +! align="center" colspan="4" | Status\n\
> +! align="center" colspan="2" | Voltage [V]\n\n\

^
                                           Ditto.
> |- bgcolor="#6699ff"\n| colspan="4" | &nbsp;\n\
> -| Probe\n| Read\n| Erase\n| Write\n\n";
> +| Probe\n| Read\n| Erase\n| Write\n\
> +| align="center" | min \n| align="center" | max\n\n";

Maybe "Min" and "Max", but lower-case is fine too, I guess.
> const char *na = "N/A";
>
> for (f = flashchips; f->name != NULL; f++)
> chipcount++;
>
> @@ -223,8 +234,13 @@ static void print_supported_chips_wiki(int cols)
> t = f->tested;
> s = flashbuses_to_text(f->bustype);
> printf("|- bgcolor=\"#%s\"\n| %s || %s || %d "
>        "|| %s || {{%s}} || {{%s}} || {{%s}} || {{%s}}\n",
> sprintf(vmin, "%0.03f", f->voltage.min/(double)1000);
> sprintf(vmax, "%0.03f", f->voltage.max/(double)1000);

Missing spaces before and after the "/".
>        f->voltage.min ? vmin : na,
>        f->voltage.min ? vmax : na);

Hm, the 'na' variable is not really needed or useful I think, just
write "N/A" twice.

Otherwise the patch is
Acked-by: Uwe Hermann

Uwe.

[PATCH 2/2] add printing of chip voltage ranges to print_wiki.c

[Stefan Tauner]

On Wed, 14 Sep 2011 23:14:42 +0200
Uwe Hermann  wrote:
> On Mon, Jun 27, 2011 at 06:30:14AM +0200, Stefan Tauner wrote:
> > static const char wiki_header[] = "= Supported devices =\n\n\
> >
> >  <div style=\"margin-top:0.5em; padding:0.5em 0.5em 0.5em 0.5em; \
> > -background-color:#eeeeee; align:right; border:1px solid #aabbcc;\"><small>\n\
> > +background-color:#eeeeee; text-align:right; border:1px solid #aabbcc;\">\
> > +<small>\n\
>
> Correct, but not sure if this was supposed to be right-aligned anyway.
> The output looks different with the patch, but it's nice too :)

i just noticed it when i was running the w3c validator on it or
opera reported the error... :)
> > -static const char chip_th[] = "{| border="0" style="font-size: smaller" \
> > -valign="top"\n|- bgcolor="#6699dd"\n! align="left" | Vendor\n\
> > -! align="left" | Device\n! align="left" | Size / kB\n\
> > -! align="left" | Type\n! align="left" colspan="4" | Status\n\n\
> > +static const char chip_th[] = "{\
> > +| border="0" style="font-size: smaller"valign="top"\n\
>                                           ^
>                 Missing space, this screws up the output.

thanks
> > +|- bgcolor="#6699dd"\n\
> > +! align="left" | Vendor\n\
> > +! align="left" | Device\n\
> > +! align="center" | Size [kB]\n\
>                         ^
>  Please replace the space with "&nbsp;" here.

actually this was on purpose. if the browser wants to put the dimension
into the next line, we should let it. we do the same thing in the -L
output btw. to safe space.
> > +! align="center" | Type\n\
> > +! align="center" colspan="4" | Status\n\
> > +! align="center" colspan="2" | Voltage [V]\n\n\
>                                          ^
>                                        Ditto.

ditto. ;)
> > |- bgcolor="#6699ff"\n| colspan="4" | &nbsp;\n\
> > -| Probe\n| Read\n| Erase\n| Write\n\n";
> > +| Probe\n| Read\n| Erase\n| Write\n\
> > +| align="center" | min \n| align="center" | max\n\n";
>
> Maybe "Min" and "Max", but lower-case is fine too, I guess.

i don't really prefer either... changed to M*
> > const char *na = "N/A";
> >
> > for (f = flashchips; f->name != NULL; f++)
> > chipcount++;
> >
> > @@ -223,8 +234,13 @@ static void print_supported_chips_wiki(int cols)
> > t = f->tested;
> > s = flashbuses_to_text(f->bustype);
> > printf("|- bgcolor=\"#%s\"\n| %s || %s || %d "
> >        "|| %s || {{%s}} || {{%s}} || {{%s}} || {{%s}}\n",
> > sprintf(vmin, "%0.03f", f->voltage.min/(double)1000);
> > sprintf(vmax, "%0.03f", f->voltage.max/(double)1000);
>
> Missing spaces before and after the "/".
> >        f->voltage.min ? vmin : na,
> >        f->voltage.min ? vmax : na);
>
> Hm, the 'na' variable is not really needed or useful I think, just
> write "N/A" twice.

those are a few relics from me wrestling with the line limit i think,
fixed. :)
> Otherwise the patch is
> Acked-by: Uwe Hermann

thanks, will commit shortly.