Re: [Flamerobin-devel] BLOB-Editor as Dialog - Patch 001

[Milan B. <mi...@pa...>]

Andreas Maier wrote:
> this patch includes the Blob-Editor:
> - wxNotebook-tab with binary (readonly) and Text-Editor

Hi Andreas,

Screenshot looks very nice. Few notes about the patch:

1. There are some extra qualifications in DataGridRows. When you're 
inside class declaration, there's no need for extra qualification. Example:

class DataGridRows
{
...	
	void DataGridRows::something();		// bad
	void something();			// good

BTW, such code fails to compile on GCC, with error message like this:

../src/gui/controls/DataGridRows.h:139: error: extra qualification 
'DataGridRows


2. when adding new files, you should add .cpp and .h file to 
flamerobin.bkl and use bakefile_gen program to regenerate the makefiles. 
We currently use Bakefile 0.2.5.

Please feel free to ask here if you have any questions/problems 
regarding these (or any other) issues.


> Maybe i have to add a "check" if the user switches from Binary to Text 
> to detect ending-0-chars.
> If not data will lost without warning. I think disabling the Text-tab if 
> blob sub-type is binary would not solve the problem anyway.

Maybe if you made the content read-only.

> What about binary data in blob-sub-type text?

> Does firebird care about blob-sub 
> types. I think no. (Correct me i i'm wrong)

You are correct.

> Personally i would prefer to "chech" data on loading. If a 0 char is 
> detected text-tab will be disabled.

I hope that by "chech" you meant "cache"? Yes, that would be very nice. 
It seems we need to discuss how will this work. Should we disable 
tab-switching until changes are saved, or we used shared memory for all 
tabs to use?

BTW, I don't like the idea of disabling, because you cannot copy/paste 
such text. It would be much better to make it read-only.

> Comments?

- IMHO, if blob subtype is text, it should open "text" tab initially.

- we should add some UI to make blob field NULL

I also have some ideas about binary tab:

- use 2 different colors for columns, so each byte is clearly separated 
from the next one. I have no idea if this would slow down wxSTC - needs 
testing

- in binary mode, rows should not have labels: 1, 2, 3, 4, 5, ..., but 
offset instead: 0, 32, 64, 96, 128, etc.

- of course, we will need to allow editing - it is BLOB *editor* after 
all. In the meantime, control should have silver/gray color to indicate 
that it is read-only.

> Thank you.

Thank you Andy. Considering this is your first patch, it is very good.

Anyway, I made a few fixes, tested on Linux/Gtk and commited everything 
to repository so we can all test and comment on it easily.


-- 
Milan Babuskov
http://www.flamerobin.org
http://www.guacosoft.com