Code::Blocks

User forums => General (but related to Code::Blocks) => Topic started by: jarro_2783 on November 05, 2007, 09:27:48 am

Title: "file updated" dialogue bug
Post by: jarro_2783 on November 05, 2007, 09:27:48 am
When a file is updated outside of codeblocks and the dialog box which asks if I want to reload the file comes up, I can hardly click the yes button because the dialog isn't tall enough.
I'm using wxGTK 2.8.4 and svn revision 4596.
Title: Re: "file updated" dialogue bug
Post by: thomas on November 05, 2007, 02:35:44 pm
Hmm... unless I am very, very, very mistaken, this is using a plain vanilla standard dialog box.

Thus, if the dialog is too small, there must be a problem with your GTK thingie..? We don't do anything really, only call the wxWidgets function which calls the respective OS function.
Title: Re: "file updated" dialogue bug
Post by: jarro_2783 on November 06, 2007, 12:37:13 am
I'm using gtk 2.10.14 which is the latest that the fedora 7 repository has, and wxGTK 2.8.4 which I compiled myself.
The height of the box seems fixed, when my file names are long enough, the text takes up more room and it pushes the buttons even further out of the dialog box. I also can't resize it.
Title: Re: "file updated" dialogue bug
Post by: JGM on November 06, 2007, 03:12:43 am
I'm using gtk 2.10.14 which is the latest that the fedora 7 repository has, and wxGTK 2.8.4 which I compiled myself.
The height of the box seems fixed, when my file names are long enough, the text takes up more room and it pushes the buttons even further out of the dialog box. I also can't resize it.

Try putting your project files into a less deep path, For example: /home/ProjectFiles or testing with shorter file names
Title: Re: "file updated" dialogue bug
Post by: jarro_2783 on November 06, 2007, 06:51:26 am
Some of the time I can see about half of the buttons. Other times when the path is longer I can see the top three pixels of the buttons.

My paths are kind of long too (/home/jarryd/programming/programs/prjfreedom/src/engine/engine.cpp for example), but the dialog should be able to resize itself shouldn't it.

Title: Re: "file updated" dialogue bug
Post by: thomas on November 06, 2007, 10:03:38 am
Just looked what we do there. It's using a wxMessageDialog which should "know" the correct size. Additionally, the dialog may be centered on the main window (if you have enabled the smart placing feature), which again only calls the standard wxWidgets CentreOnWindow function under all non-Windows platforms.

The only time the dialog's size can modified at all is if you have enabled the smart placement functionality in constraint mode, and if your screen is so small that the dialog would not fit. However, you cannot even do that any more, since someone removed this option from the settings panel  8)

So... clueless as to what happens there with you.
Title: Re: "file updated" dialogue bug
Post by: dmoore on November 06, 2007, 05:33:43 pm
I have also had this problem on ubuntu feisty
Title: Re: "file updated" dialogue bug
Post by: jens on November 06, 2007, 06:46:03 pm
The following patch fixes the bug.
Code: [Select]
--- codeblocks-1.0svn.orig/src/sdk/resources/confirm_replace.xrc        2007-09-28 13:55:10.000000000 +0000
+++ codeblocks-1.0svn.work/src/sdk/resources/confirm_replace.xrc        2007-11-06 13:28:21.000000000 +0000
@@ -12,6 +12,7 @@
         </object>
         <flag>wxALL|wxGROW</flag>
         <border>8</border>
+        <option>1</option>
       </object>
       <object class="sizeritem">
         <object class="wxBoxSizer">
--- codeblocks-1.0svn.orig/src/sdk/resources/confirm_replace_multiple.xrc       2007-09-28 13:55:10.000000000 +0000
+++ codeblocks-1.0svn.work/src/sdk/resources/confirm_replace_multiple.xrc       2007-11-06 13:31:14.000000000 +0000
@@ -11,6 +11,7 @@
         </object>
         <flag>wxALL|wxGROW</flag>
         <border>8</border>
+        <option>1</option>
       </object>
       <object class="sizeritem">
         <object class="wxBoxSizer">

I added the line "<option>1</option>" to "confirm_replace_multiple.xrc" also, eve if it is not used by the fileupdate-Dlg, to make both behave identical.

If you confirm the patch, I can post it at berlios.

On windows the problem with extremly long file-name (-paths) is different, because the line is not wrapped there, and a part of it get cut by the Dialog-Border.
Title: Re: "file updated" dialogue bug
Post by: thomas on November 07, 2007, 09:28:22 am
The following patch fixes the bug.
Are you sure? The "reload file?" box uses no xrc at all.
Your patch seems to address something different (search/replace). Apparently those dialogs are too small on our system too?
Title: Re: "file updated" dialogue bug
Post by: jens on November 07, 2007, 09:58:04 am
The following patch fixes the bug.
Are you sure? The "reload file?" box uses no xrc at all.
Your patch seems to address something different (search/replace). Apparently those dialogs are too small on our system too?

Yes, I'm sure !
The "cbMessageBox" is used when a file is deleted or moved :

editormanager.cpp (line 988s to 992)
Code: [Select]
            msg.Printf(_("%s has been deleted, or is no longer available.\n"
                         "Do you wish to keep the file open?\n"
                         "Yes to keep the file, No to close it."), ed->GetFilename().c_str());
            if (cbMessageBox(msg, _("File changed!"), wxICON_QUESTION | wxYES_NO) == wxID_YES)
                ed->SetModified(true);

If a file gets modified outside the editor, this code is used :

editormanager.cpp (lines 1036 to 1038)
Code: [Select]
                msg.Printf(_("File %s is modified outside the IDE...\nDo you want to reload it (you will lose any unsaved work)?"),
                           ed->GetFilename().c_str());
                ConfirmReplaceDlg dlg(Manager::Get()->GetAppWindow(), false, msg);

The bug (I think it is a bug) only happens for me if I use very long file-/pathnames. I tested it.
The code with the "cbMessageBox" works for me, even with really deep nested paths.
Title: Re: "file updated" dialogue bug
Post by: thomas on November 07, 2007, 10:20:27 am
Yes, I'm sure !
The "cbMessageBox" is used when a file is deleted or moved
Yes, that is what I'm saying... it does not use XRC at all. cbMessageBox calls wxMessageDialog (which, again, does not use XRC). So I am confused how this can be fixed by a patch to an XRC file.
Title: Re: "file updated" dialogue bug
Post by: jens on November 07, 2007, 10:34:23 am
Yes, I'm sure !
The "cbMessageBox" is used when a file is deleted or moved
Yes, that is what I'm saying... it does not use XRC at all. cbMessageBox calls wxMessageDialog (which, again, does not use XRC). So I am confused how this can be fixed by a patch to an XRC file.

It can be fixed, because the "file updated"-dialog does not use "cbMessageBox", but "ConfirmReplaceDlg" !
That's what I posted :
If a file gets modified outside the editor, this code is used :

editormanager.cpp (lines 1036 to 1038)
Code: [Select]
                msg.Printf(_("File %s is modified outside the IDE...\nDo you want to reload it (you will lose any unsaved work)?"),
                           ed->GetFilename().c_str());
                ConfirmReplaceDlg dlg(Manager::Get()->GetAppWindow(), false, msg);

The bug (I think it is a bug) only happens for me if I use very long file-/pathnames. I tested it.
The code with the "cbMessageBox" works for me, even with really deep nested paths.
Title: Re: "file updated" dialogue bug
Post by: MortenMacFly on November 07, 2007, 10:51:57 am
It can be fixed, because the "file updated"-dialog does not use "cbMessageBox", but "ConfirmReplaceDlg" !
This is not a good solution at all. This dialog has nothing to do with the content you want to use it with. The issue here is that the filename is in the title bar of the dialog (cbMessageBox). This is very bad as the dialog cannot wrap a single-line label. The solution I'd propose is plain simple:
Put the filename in the message box content and (maybe) remove or at least shorten the path. That's it.
With regards, Morten.
Title: Re: "file updated" dialogue bug
Post by: jens on November 07, 2007, 11:18:38 am
It can be fixed, because the "file updated"-dialog does not use "cbMessageBox", but "ConfirmReplaceDlg" !
This is not a good solution at all. This dialog has nothing to do with the content you want to use it with. The issue here is that the filename is in the title bar of the dialog (cbMessageBox). This is very bad as the dialog cannot wrap a single-line label. The solution I'd propose is plain simple:
Put the filename in the message box content and (maybe) remove or at least shorten the path. That's it.
With regards, Morten.

First: I don't have any problems with long file-/pathnames.
But the OP has.

Second: the dialog is not used by me, it is original C::B-code. I think it might be because of the "All"-button.

Third: on linux the filepaths are wrapped at the "/", on W2K (I did not try on XP) the paths are not wrapped (maybe because  of the "\").

Fourth: where is the problem? The "<option>1</option>" makes the wxStaticText-control vertical stretchable. This only happens if the content does not fit in it and this is what I would suggest.

I still think it is a good solution as long as "ConfirmReplaceDlg" is used.
Title: Re: "file updated" dialogue bug
Post by: MortenMacFly on November 07, 2007, 11:21:16 am
I still think it is a good solution as long as "ConfirmReplaceDlg" is used.
I'm sorry, but within the content we are talking about this is not an option.

Edit: For clarification: Hijacking foreign dialogs and mis-using them within a foreign content is no good.
Title: Re: "file updated" dialogue bug
Post by: jens on November 07, 2007, 11:41:05 am
I still think it is a good solution as long as "ConfirmReplaceDlg" is used.
I'm sorry, but within the content we are talking about this is not an option.

Edit: For clarification: Hijacking foreign dialogs and mis-using them within a foreign content is no good.

Did anybody really read what I post?

I try to make it a little bit clearer.

There is existing code in editormanager.cpp, not written by me.
I don't know who the author is and I think this doesn't matter.
The fact is: this code is there and it is used.

I also think that Hijacking code is not good, because if the author of the "stolen" code changes it, it might break something.
But: I didn't do that, it was a C::B-developer, or a C::B-developer decided to put in into svn!

No problems with that.

The changes to the xrc-files on the other hand do not break the original-code. The only change the original behaviour, when the label contains too long text. This is a standard-beaviour when using sizers in wxWidgets, and that is one of the main causes to use them.

The clean solution would be to rewrite the editormanager-code.

Perhaps I will do so, but I don't have enaough time to do so until (perhaps) the next weekend.

Title: Re: "file updated" dialogue bug
Post by: jens on November 07, 2007, 11:46:59 am
... or at least shorten the path. That's it.

By the way, it's not always the developers decision which path he uses.
And I don't think an IDE should force the user to use special paths, if there is no really good cause to do that.
Title: Re: "file updated" dialogue bug
Post by: MortenMacFly on November 07, 2007, 11:51:09 am
Did anybody really read what I post?
We all did but you are missing the point here.

We talk about the code that informs about files being modified outside the C::B IDE. This has *nothing* (I repeat: *nothing*) to do with any search-and-replace operation. So we will *not* use code/dialogs/whatever (even if it's a XRC file) that is developed for the purpose of search-and-replace within the context we are talking about. Period.

With regards, Morten.
Title: Re: "file updated" dialogue bug
Post by: thomas on November 07, 2007, 12:00:10 pm
Quote
Did anybody really read what I post?
Read yes, understand no. I think you are talking about an entirely different dialog.
Quote
There is existing code in editormanager.cpp, not written by me.
Yes, this code uses cbMessageBox. It does not use XRC. And, it is a different dialog than the one you want to patch.

Quote
The changes to the xrc-files on the other hand do not break the original-code. The only change the original behaviour, when the label contains too long text. This is a standard-beaviour when using sizers in wxWidgets, and that is one of the main causes to use them.
Yes, nobody doubts this... but it is a completely unrelated dialog.

Quote
The clean solution would be to rewrite the editormanager-code.
The code in editormanager.cpp is clean, it does not need a rewrite. EditorManager calls a standard function which takes a text and shows a "Yes/No" dialog box using standard components (wxMessageDialog). That is a 100% legitimate thing to do.
I don't know why it does not work properly for the OP, but it surely does not need any rewrite inside EditorManager.
Title: Re: "file updated" dialogue bug
Post by: jens on November 07, 2007, 12:07:33 pm
Did anybody really read what I post?
We all did but you are missing the point here.

We talk about the code that informs about files being modified outside the C::B IDE. This has *nothing* (I repeat: *nothing*) to do with any search-and-replace operation. So we will *not* use code/dialogs/whatever (even if it's a XRC file) that is developed for the purpose of search-and-replace within the context we are talking about. Period.

With regards, Morten.

Sorry, but that's not right.

Open this (http://svn.berlios.de/viewcvs/codeblocks/trunk/src/sdk/editormanager.cpp?view=markup&rev=4600) link from the C::B-svn.
Search for the function "CheckForExternallyModifiedFiles()".
In the function you will find this code:
Code: [Select]
        //File content changed?
        if (last.IsLaterThan(ed->GetLastModificationTime()))
            b_modified = true;

        if (b_modified)
        {
            // modified; ask to reload
            int ret = -1;
            if (!reloadAll)
            {
                wxString msg;
                msg.Printf(_("File %s is modified outside the IDE...\nDo you want to reload it (you will lose any unsaved work)?"),
                           ed->GetFilename().c_str());
                ConfirmReplaceDlg dlg(Manager::Get()->GetAppWindow(), false, msg);
                dlg.SetTitle(_("Reload file?"));
                PlaceWindow(&dlg);
                ret = dlg.ShowModal();
                reloadAll = ret == crAll;
            }
            if (reloadAll || ret == crYes)
            {
                if (!ed->Reload())
                    failedFiles.Add(ed->GetFilename());
            }
            else if (ret == crCancel)
                break;
            else if (ret == crNo)
                ed->Touch();
        }

It includes this line:

     ConfirmReplaceDlg dlg(Manager::Get()->GetAppWindow(), false, msg);

That's the fact, nothing else.


Edit:
It first came up in svn299 as you can see in the diff (http://svn.berlios.de/viewcvs/codeblocks/trunk/src/sdk/editormanager.cpp?rev=299&r1=289&r2=299).
Title: Re: "file updated" dialogue bug
Post by: jarro_2783 on November 07, 2007, 01:22:59 pm
I agree with jens.
editormanager.cpp is definitely creating a ConfirmReplaceDlg for that dialog box. If that's using an XRC file then the patch looks alright.
Title: Re: "file updated" dialogue bug
Post by: thomas on November 07, 2007, 02:07:25 pm
Ah, see... I told you that you were talking of a different dialog. I was talking about the one 15 lines earlier (in the same function), which is why I had no idea what the heck you were talking about :)

You're right, this one uses ConfirmReplaceDlg for some reason. Funny... I don't see why that one couldn't be a cbMessageDialog like in the other one.
Title: Re: "file updated" dialogue bug
Post by: MortenMacFly on November 07, 2007, 03:48:48 pm
Funny... I don't see why that one couldn't be a cbMessageDialog like in the other one.
Agreed. I think we have discovered a bug or at least very bad design. Thanks for clarifying.
Title: Re: "file updated" dialogue bug
Post by: dmoore on November 07, 2007, 04:25:11 pm
Funny... I don't see why that one couldn't be a cbMessageDialog like in the other one.
Agreed. I think we have discovered a bug or at least very bad design. Thanks for clarifying.

well if you look at the code, the user is given the following options when a file has been modified:
* Yes (Reload)
* No (Don't Reload and make the internal date in the editor the same as the file)
* Cancel (Don't reload and don't change the internal date, break out of the loop)
* All (Reload all changed files)

cbMessageBox can't do that.

The earlier use of cbMessageBox concerns deleted files. Arguably, there are only two reasonable options: Yes - keep the contents of the open editor, No - close the editor.

As someone who has been annoyed by this bug on Linux in that past I'd like to see it fixed. I think Jens' patch does that without breaking anything, so why wouldn't it be accepted?
Title: Re: "file updated" dialogue bug
Post by: MortenMacFly on November 07, 2007, 04:38:42 pm
As someone who has been annoyed by this bug on Linux in that past I'd like to see it fixed. I think Jens' patch does that without breaking anything, so why wouldn't it be accepted?
Because it leaves a pointer to a "bug" that needs to be fixed... ;-) On the other hand... let's see...