News:

When registered with our forums, feel free to send a "here I am" post here to differ human beings from SPAM bots.

Main Menu

Bug in 'swap header / source'

Started by ironhead, October 14, 2009, 05:01:59 AM

Previous topic - Next topic

ironhead

Quote from: Zini on October 20, 2009, 03:58:48 PM
Still haven't got around to test your patch (sorry, so busy atm. its insane!), but IIRC it stops the feature from working when h and cpp files sit in different directories. That would break the feature completely for me (and most likely a lot of other people).

I think it all depends on your file structure, since the existing implementation also breaks if you have multiple subdirectories with source and include directories in the subdirectories.

QuoteI agree that a solutation that handles all cases isn't possible without storing information about the source layout in the project file. However the situation you mentioned is handled perfectly by my algorithm. Did you have to time to take a look at it?

I'm in much the same position as you at the moment unfortunately (having very little time to work on things), hopefully sometime early next week I can take a look at implementing it.  If you have some time before that and feel inclined at taking a stab at it, I'd be happy to help out. :D

Zini

QuoteI think it all depends on your file structure, since the existing implementation also breaks if you have multiple subdirectories with source and include directories in the subdirectories.

Not really. I am currently on SVN rev 5679. The swap feature works here most of the time (without automatically creating missing files though), unless there is more than one file with the same name (which has never worked properly with any version of Code::Blocks). That is still better than to break the feature entirely for certain layouts.

Zini

QuoteIf you have some time before that and feel inclined at taking a stab at it, I'd be happy to help out.

Rather unlikely. I just had another look at the code. The time I would need to familiarize myself with WxWidgets API (never worked with it before) and the relevant Code::Blocks API (know nothing about it either) would most likely exceed any amount of time I can make available.
However I might be able to refine the algorithm a bit, if you are willing to try implementing it. I have a few more ideas, that should cover some additional edge-cases.

Zini

There we go ...

listCandidates (file):
   e = extension (file)
   if e in header_extension_list:
       extension_list = source_extension_list
   elif e in source_extension_list:
       extension_list = header_extension_list
   else:
       raise error (unknown extension)

   l = leaf_without_extension (file)
       
   for f in project: # iterate over all files in the project
       if leaf_without_extension (f)==l and extension (f) in extension_list:
           add f to list
           
   return list

findCandidate (file, list):
   if not strictMode and len (list)==1:
       return list[0]
   
   n = len (file) # len (file) is the number of path elements

   if n>1: # search for shared path elements
       for f in list if len (f)==n:
           for i in range (n-1): # for loop over the first n-1 integers starting with 0
               if f[i]==file[i]:
                   return f # success
   else # name consists of leaf elements only
       for f in list if len (f)==1:
           return f # success (top level directory)

   # maybe add other methods to determine the file in case the above fails

   return null # no file found; either does match no known pattern or it does not exist
   
swap (file):
   list = listCandidates (file)

   c = findCandidate (file, list):
   
   if c!=null:
       open c
       return
       
   # file does not exist -> try to find another file pair
   parent = file[0:n-2] # one level up (the directory where the file is located)
   for f in parent if f!=file: # iterating over the files in the directory
       c = findCandidate (f, list)
       if c!=null:
           # may want to include a sanity check here: warn or abort if file already exists
           create corresponding file in directory c[0:n-2] and open it
           return
           
   # give up; either ask the user for a directory or report that the file can't be created


Optimized, better documented and with a little bug fix. This should work.

Regarding the strictMode variable: This should be either a Code.:Blocks-wide or a project-wide setting. In strict mode the algorithm will not handle some less symmetric source layouts correctly. In non-strict mode multiple files with the same name won't be handles correctly, if one of the files still needs to be created. I think that is a good compromise.

Edit: File names are assumed to be relative to the project directory.

ironhead

Cool, thank you for the detailed pseudo code.  I'll take a look at implementing it next week (I have a couple of deadlines to take care of this week).

Cheers!

CuteAlien

Testing this with latest nightly build (svn 5911) it seems that currently c::b always prefers switching to a header/source which is already opened. If several files with a fitting name are open then it switches to the one which was opened first. Regardless if those files are in the same project or folder. It only works correct (for my case) when no file with a fitting name is already opened.

ironhead

The behaviour on the trunk is still unchanged.  I unfortunately have not had time to work on a new version of the patch.  If someone else has time and is willing to take a stab at it, I'd be happy to help out.

MortenMacFly

Please have a look at Patch #2803 at BerliOS patch tracker. From my point of view this has most chances to implement a "correct" behaviour. I am testing this for a while now. Most likely it solves your problems.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

oBFusCATed

Sorry for resurrecting this topic, but I wanted to make this feature work for file where the case doesn't match.
The code seems pretty complex and I don't won't to change it, is anyone willing to implement such feature?

My current problem is that these to files: myfile.h and MyFile.cpp are not swapped correctly.

The example project is also missing.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Alpha

Maybe something like this?  (Warning, not yet heavily tested.)

Index: src/sdk/editormanager.cpp
===================================================================
--- src/sdk/editormanager.cpp (revision 8646)
+++ src/sdk/editormanager.cpp (working copy)
@@ -1014,8 +1014,8 @@

bool EditorManager::IsHeaderSource(const wxFileName& candidateFile, const wxFileName& activeFile, FileType ftActive)
{
-    // Verify the base name mathes
-    if (candidateFile.GetName() == activeFile.GetName())
+    // Verify the base name matches
+    if (candidateFile.GetName().CmpNoCase(activeFile.GetName()) == 0)
     {
         // Verify:
         // If looking for a header we have a source OR

p2rkw

#40
Hmm... No. Only windows isn't case sensitive.

Btw. Newly opened file tab (via F11) might be placed next to current file, not at end.

MortenMacFly

Quote from: p2rkw on December 11, 2012, 11:12:03 PM
Hmm... No. Only windows isn't case sensitive.
That's not correct, too.

The right way is to first look for a case sensitive and then as backup for a case non-sensitive on Linux platforms. On Windows, you can start with case-insensitive directly. So - these are two cases that cannot be handled together.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

Alpha

Quote from: MortenMacFly on December 12, 2012, 08:49:24 AM
The right way is to first look for a case sensitive and then as backup for a case non-sensitive on Linux platforms. On Windows, you can start with case-insensitive directly. So - these are two cases that cannot be handled together.
Unless someone already has begun work on this, I will see if I can pull something together.

Alpha

Quote from: Alpha on December 15, 2012, 11:16:43 PM
I will see if I can pull something together.
I believe this meets the criteria, and it passes my test cases.  oBFusCATed, does it work on your project?

Index: src/sdk/editormanager.cpp
===================================================================
--- src/sdk/editormanager.cpp (revision 8669)
+++ src/sdk/editormanager.cpp (working copy)
@@ -1012,11 +1012,14 @@
     m_isCheckingForExternallyModifiedFiles = false;
}

-bool EditorManager::IsHeaderSource(const wxFileName& candidateFile, const wxFileName& activeFile, FileType ftActive)
+bool EditorManager::IsHeaderSource(const wxFileName& candidateFile, const wxFileName& activeFile, FileType ftActive, bool& isCandidate)
{
-    // Verify the base name mathes
-    if (candidateFile.GetName() == activeFile.GetName())
+    // Verify the base name matches
+    if (candidateFile.GetName().CmpNoCase(activeFile.GetName()) == 0)
     {
+        // non-Windows platforms: case-insensitive match -> isCandidate
+        isCandidate = !( platform::windows || (candidateFile.GetName() == activeFile.GetName()) );
+
         // Verify:
         // If looking for a header we have a source OR
         // If looking for a source we have a header
@@ -1056,13 +1059,12 @@
     {
         wxFileName currentCandidateFile(candidateFilesArray[i]);

-        if (IsHeaderSource(currentCandidateFile, activeFile, ftActive))
+        if (IsHeaderSource(currentCandidateFile, activeFile, ftActive, isCandidate))
         {
             bool isUpper = wxIsupper(currentCandidateFile.GetExt()[0]);
-            if (isUpper == extStartsWithCapital)
+            if (isUpper == extStartsWithCapital && !isCandidate)
             {
                 // we definitely found the header/source we were searching for
-                isCandidate = false;
                 return currentCandidateFile;
             }
             else
Index: src/include/editormanager.h
===================================================================
--- src/include/editormanager.h (revision 8669)
+++ src/include/editormanager.h (working copy)
@@ -194,7 +194,7 @@
         int FindInFiles(cbFindReplaceData* data);
         int Replace(cbStyledTextCtrl* control, cbFindReplaceData* data);
         int ReplaceInFiles(cbFindReplaceData* data);
-        bool IsHeaderSource(const wxFileName& candidateFile, const wxFileName& activeFile, FileType ftActive);
+        bool IsHeaderSource(const wxFileName& candidateFile, const wxFileName& activeFile, FileType ftActive, bool& isCandidate);
         wxFileName FindHeaderSource(const wxArrayString& candidateFilesArray, const wxFileName& activeFile, bool& isCandidate);

         cbAuiNotebook*             m_pNotebook;