Some UI refactoring/tidy up

[dmoore]

Adding to the original list, here are some more paper cuts with Dialogs and default/escape actions:

7. OK/Close is not default, escape does not close the dialog
select target dialog
file properties
project properties
manage plugins
settings environment
settings editor
settings compiler
settings debugger
settings global variables

8. Escape key does not close the dialog
project build options
file print dialog
add todo item dialog
replace prompt (despite setting SetEscapeId in the constructor)
browse tracker settings (from edit menu)
categories & file types
project remove files

9. too large for small screens
all(?) file picker dialogs (e.g. file open, file save as, project add files, ...)

10.  broken layout
settings global variables (need to scroll dialog, probably just missing a layout/fit call)

This is on Linux, not sure there are the same problems on windows. Let me know if I have misdiagnosed or miss some issues. The fixes for 7 and 8 may not be straightforward because they may be due to bugs in wxWidgets.

[MortenMacFly]

I had used svn cp to do this, but the svn diff does not show up the need to copy replacedlg.* first.

What you need to do is copy these files plain, the svn add them temporarily, then do the modifications and then create the patch.

Both patches presented here do not work for me. Alternatively just provide the two new files as "plain" files here, not included in the patch.

[dmoore]

I had used svn cp to do this, but the svn diff does not show up the need to copy replacedlg.* first.

What you need to do is copy these files plain, the svn add them temporarily, then do the modifications and then create the patch.

Both patches presented here do not work for me. Alternatively just provide the two new files as "plain" files here, not included in the patch.

Did you make the copies using the two commands I suggested? (The bigger problem may be because the patch was created on linux and you are trying to apply to windows)

[MortenMacFly]

Did you make the copies using the two commands I suggested?

Yes, I did - but they were left un-modifed after applying the patch.

[MortenMacFly]

Yes, I did - but they were left un-modifed after applying the patch.

BTW, in the patch file you see this:
--- src/sdk/findreplacedlg.cpp   (revision 8239)
+++ src/sdk/findreplacedlg.cpp   (working copy)

...but this will fail, because in revision 8239 there is no such file. That's why I said you need to add them.

[Jenna]

(The bigger problem may be because the patch was created on linux and you are trying to apply to windows)

If they are created with "svn diff", they normally work out of the box on windows.

[MortenMacFly]

If they are created with "svn diff", they normally work out of the box on windows.

Yes, unless there are missing steps. Like here, patch assumes files to be present which are not. So it cannot work. Not even, if you do a svn cp, because they are assumed to be there, committed, in r8239 - that's not the case.

[dmoore]

I guess you are using some "fancy" patch tool. good ole linux patch apparently ignores the stuff in parentheses, so it applies fine after creating the copies of the file.

I've attached another one, hopefully this works. (It does say "revision 0" for the new files, hope that is what you are expecting)

@jens: yes, used svn diff.

I realize that I need to disable the option to convert line endings for multiline "find", because I don't think it makes sense. Maybe multiline search doesn't make sense at all for "find" (I figure that it will be occasionally useful so worth keeping).

[MortenMacFly]

I've attached another one, hopefully this works. (It does say "revision 0" for the new files, hope that is what you are expecting)

Yes, this one worked out of the box (as expected).

I guess you are using some "fancy" patch tool. good ole linux patch apparently ignores the stuff in parentheses, so it applies fine after creating the copies of the file.

Well I am on Windows - so thee is no "good ole linux patch". And still: A patch is correct only, if it doesn't require any pre-requisite. Everything else just may work but is not guaranteed.

[dmoore]

Yes, I did - but they were left un-modifed after applying the patch.

BTW, in the patch file you see this:
--- src/sdk/findreplacedlg.cpp   (revision 8239)
+++ src/sdk/findreplacedlg.cpp   (working copy)

...but this will fail, because in revision 8239 there is no such file. That's why I said you need to add them.

So OK I was wrong, but isn't that partly an issue with svn diff? Why does it treat the added file as if it was in revision 8239, when clearly it wasn't? I used svn cp, but I guess diff just treats the cp as having occurred and just reports the changes.

[MortenMacFly]

So OK I was wrong, but isn't that partly an issue with svn diff? Why does it treat the added file as if it was in revision 8239, when clearly it wasn't? I used svn cp, but I guess diff just treats the cp as having occurred and just reports the changes.

Maybe... I wonder how "svn cp" and "svn add" are interpreted by patch. I mean: In both cases you have a brand new file. From a "diff"-view perspective its the same. But: If you look at both patches, in one case you get a diff to an existing file, in the other you get the full file. I guess its just a mystery...

[dmoore]

There is some unconvincing rationalization of the issue here. They mention a "treat copies as adds" option, but if default behavior produces broken patches I would call that a bug.

Anyway, any thoughts on the patch? (Note that the patch does not remove the files that are no longer needed)

[MortenMacFly]

Anyway, any thoughts on the patch? (Note that the patch does not remove the files that are no longer needed)

Hey, I've just managed to compile C::B... please gimme more than 5 minutes to try...
BTW: Which files should I remove now? (Header, source, resource files?!)

[dmoore]

Sorry ...

BTW: Which files should I remove now? (Header, source, resource files?!)

It will run as is, but eventually will need to remove find_dialog.xrc, finddialog.cpp, finddlg.h, finddlg.cpp, replacedlg.cpp, replacedlg.h (in retrospect a smarter way to create the patch would be to svn mv replacedlg to findreplacedlg)
Could also rename replace_dialog.xrc find_replace_dialog.xrc

Will then need to adjust project/make files accordingly

[MortenMacFly]

Will then need to adjust project/make files accordingly

Well, when compiling the C::B sources with the patch applied, compilation of the plugins (namely codesnippets) will fail:
C:/Devel/CodeBlocks/src/plugins/contrib/codesnippets/editor/seditormanager.cpp:1524:101: error: cannot allocate an object of abstract type 'FindDlg'
C:/Devel/CodeBlocks/src/plugins/contrib/codesnippets/editor/seditormanager.cpp:1527:104: error: cannot allocate an object of abstract type 'ReplaceDlg'

Can you provide a complete patch, so one can compile the whole workspace?