Re: vesafb terminal for testing.

[Marco Gerards <metgerards@student.han.nl>]

Vesa Jääskeläinen <chaac@nic.fi> writes:

> Marco Gerards wrote:
>> Vesa Jääskeläinen <chaac@nic.fi> writes:
>> 
>>>> There are some long lines, can you please make them shorter so
>>>> the maximal line length is 78 characters?
>>> Hmm... I made another scan for those. Still I left some of those there
>>> because in my opinion it is much more readable in that way. But splitted
>>> some lines.
>> 
>> Can you should me which so I can come up with a suggestion?
>
> video/i386/pc/vbe.c:
> - setpixel functions. There are couple of "too" long lines.
> - grub_vbe_get_video_mode_info first variable definition is a bit long.

      grub_uint32_t *ptr = (grub_uint32_t *)(framebuffer + y * bytes_per_scan_line + x * 4);

This can be changed in:

      grub_uint32_t *ptr;
      ptr = (grub_uint32_t *)(framebuffer + y * bytes_per_scan_line + x * 4);

or:

      grub_uint32_t *ptr = (grub_uint32_t *)(framebuffer + y *
                                             bytes_per_scan_line
                                             + x * 4);


> term/i386/pc/vesafb.c:
> - grub_virtual_screen_setup, grub_malloc line.

You fixed this already.  Please put operators on the beginning of the
next line instead of the end of the line.

> - write_char, set pixel line at end.

How about:

          grub_vbe_set_pixel_index(i + (virtual_screen.cursor_x
                                        * virtual_screen.char_width),
                                   y + (virtual_screen.cursor_y
                                        * virtual_screen.char_height),
                                   color);


> - grub_virtual_screen_setcolor, function definition.

This is a tough one, but not important I guess. :)


> commands/i386/pc/vbe_test.c:
> - GRUB_MOD_INIT.

You can split the line after the commas.

> commands/i386/pc/vbe_list_modes.c:
> - GRUB_MOD_INIT.

Same here.

> include/grub/i386/pc/vbe.h:
> - several function prototypes.

You could try to split the line after the commas for the arguments.
Sometimes it is not possible to create a shorter line and in that case
it is not important.

>>> I will scan the code once more and then commit a bit different version
>>> to CVS as Okuji asked to commit it to there for easier testing.
>
> Just a note until admins fix those commit messages, first version of vbe
> terminal support is now committed. It still needs more work, but it is a
> starting point.

Cool!  If you do receive the commit messages, can you forward them so
we could see them as well?

Thanks,
Marco