Request for review: Okteta plugin 0.1.0 for KDevelop 4.0

[Friedrich W. H. Kossebau]

(CC:ing Alex, author of the Structures tool...)

Hi Andreas,

thanks for review :)

Mardi, le 24 août 2010, à 20:40, Andreas Pakulat a écrit:
> Using a new mail for my notes on this. I'm writing this down as I go
> through and explore the features (as far as I can):
> 
> - Wouldn't the "Byte Table" be better named something with Ascii or
>   Latin1 in it? Because thats what it is after all right? Or is the
>   char-column to be considered just 'one among several representations'?

Well, Ascii/Latin1 are just two 7/8-bit charsets among many :)
But you are right, "Byte Table" is a bad name. The table gives an overview 
about the possible values a byte can take in different value codings and the 
corresponding char in, hm, indeed only the charset currently used in the view, 
and not in more.
Would "Value/Char Table" be better?
Perhaps could use also only the current value coding, not all four available.

> - there's some #if 0'ed code that looks like copied from the designer
>   plugin, probably should be removed?

Will be replaced by a proper comment, okay.

> - there's a rendering glitch for the address bar on the left of the
>   document view

? Could you help with a screenshot what you mean?

> - The structures toolview seems to analyze the document from the cursor
>   position which was kinda un-intuitive when opening it the first time
>   as the structure information for my elf binary was wrong. Also it
>   might be good to auto-enable the elf+png stuff, took me a bit to
>   figure out how to get anything into the view :)

Alex, what do you think? Andreas could have a point here.
 
> - The strings toolview didn't seem to do anything, everything was
>   disabled even after typing something into the lineedit.
> 
> - The statistics toolview also seems to be not really useful. The button
>   is disabled and the table doesn't show anything.

You first have to select some data to use the tools on :)
I once decided against auto-selection of everything, but it seems not 
intuitive for some tools for some people. Will have to reconsider, if there 
are more complains.

> - I think it would be nice to have no separate "Open as ByteArray"
>   option in the context menu but instead have that as an option under
>   "Open With". But thats something we'll have to tackle in kdevplatform
>   first :)

Yes, is just a workaround :)

> Codewise it looks quite clean and nicely designed.

Fine, thanks.

> I think if you can fix that rendering bug and make the two toolviews I
> noted do something useful (maybe I'm just too stupid to use them
> correctly??) or remove them its good to go for a release.

Need to be shown the rendering bug first, sorry. The problem with the two 
tools (also valid for all other tools) is hopefully explained.

Who would do the import into the kdevelop git repo? Would be nice it history 
could be preserved...

Cheers
Friedrich
-- 
KDE Okteta - a simple hex editor - http://utils.kde.org/projects/okteta