Author Topic: "file updated" dialogue bug  (Read 14200 times)

Offline jens

  • Administrator
  • Lives here!
  • *****
  • Posts: 7265
    • Jens' unofficial debian-repository for the Code::Blocks - IDE
Re: "file updated" dialogue bug
« Reply #15 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.


Offline jens

  • Administrator
  • Lives here!
  • *****
  • Posts: 7265
    • Jens' unofficial debian-repository for the Code::Blocks - IDE
Re: "file updated" dialogue bug
« Reply #16 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.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9496
Re: "file updated" dialogue bug
« Reply #17 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.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: "file updated" dialogue bug
« Reply #18 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.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline jens

  • Administrator
  • Lives here!
  • *****
  • Posts: 7265
    • Jens' unofficial debian-repository for the Code::Blocks - IDE
Re: "file updated" dialogue bug
« Reply #19 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 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.

Offline jarro_2783

  • Multiple posting newcomer
  • *
  • Posts: 99
    • Project Freedom
Re: "file updated" dialogue bug
« Reply #20 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.

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: "file updated" dialogue bug
« Reply #21 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.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9496
Re: "file updated" dialogue bug
« Reply #22 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.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: "file updated" dialogue bug
« Reply #23 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?
« Last Edit: November 07, 2007, 04:27:20 pm by dmoore »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9496
Re: "file updated" dialogue bug
« Reply #24 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...
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ