Python Code Completion

[ollydbg]

Code Select Expand

+bool FortranProject::IsProviderFor(cbEditor *cb)
+{
+    if (!ed
+        || !m_pNativeParser->IsFileFortran(ed->GetShortName())
+        || !Manager::Get()->GetConfigManager(_T("fortran_project"))->ReadBool(_T("/use_code_completion"), true))
+    {
+        return;
+    }
+}
+
return true or false?

[dmoore]

oops  ;D

false

and return true outside the block.

[MortenMacFly]

Here's a revised patch to try. I've made the needed change to the fortran plugin as well.

This patch does not apply for me. :-\ Furthermore, it won't work with Fortran as you provide a cbEditor* cb via the interface and then use "ed->" internally.

However, after applying this patch manually and adjusting the tiny issues I am now testing...

[dmoore]

However, after applying this patch manually and adjusting the tiny issues I am now testing...

As should now be clear I didn't test the FORTRAN bits, because I have never used it and didn't have a FORTRAN project handy. (I did test analogous patch for the python code completer which seemed to be doing the right thing)

[MortenMacFly]

As should now be clear I didn't test the FORTRAN bits

Maybe darmar can do some trials, too. He might have something to contribute... I'll send him a PM.

[darmar]

Here's a revised patch to try. I've made the needed change to the fortran plugin as well.

I would like to explain how the same thing is functioning now (without this patch).

Code Select Expand


Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 8464)
@@ -727,8 +727,12 @@
            if (   ParserCommon::FileType(filename) == ParserCommon::ftOther
                && Manager::Get()->GetPluginManager()->IsFileExtRegistered(filename) )
                return;


The idea is the same as in dmoore's patch. The function "IsFileExtRegistered" is doing similar thing as "IsProviderFor(ed)" in this patch.
Every CC plugin should register file extensions it like to handle. In FortranProject plugin the corresponding code is:

Code Select Expand


void FortranProject::RegisterFileExtensions()
{
    PluginManager* plugman = Manager::Get()->GetPluginManager();
    StringSet fileExts;
    m_pNativeParser->GetFortranFileExts(fileExts);
    plugman->RegisterCCFileExts(_T("FortranProject"), fileExts);
}



So, Python CC should register file extensions it like to handle with "plugman->RegisterCCFileExts(_T("PythonCC"), fileExts);".

If developers prefer to apply this patch, it is OK from FortranProject plugin's point of view. You just need to delete Rev 7922 patch (in this patch the required changes were made in C::B API and CC plugin).

[danselmi]

Aren't these the same extensions as used for Syntax highlighting? (configured in Settings->Editor->SyntaxHighlighting->Filemasks)

[MortenMacFly]

Aren't these the same extensions as used for Syntax highlighting? (configured in Settings->Editor->SyntaxHighlighting->Filemasks)

Yes, ...so what? Id that bad? ???

The only issue I see here is that oe can change the nbame of the file group. Like I did for example with "Sources" which I renamed to "C/C++ Sources" and "Fortran Sources" to group these files. As the categories can change this may break functionality, see for example:
FileType FileTypeOf(const wxString& filename) in globals.cpp
Here I had to change:

Code Select Expand

                   if (fgm->GetGroupName(i) == _T("Sources") && fgm->MatchesMask(ext, i))
                       return ftSource;
                   if (fgm->GetGroupName(i) == _T("Headers") && fgm->MatchesMask(ext, i))
                       return ftHeader;

...to:

Code Select Expand

                   if (fgm->GetGroupName(i).Contains(_T("Source")) && fgm->MatchesMask(ext, i))
                       return ftSource;
                   if (fgm->GetGroupName(i).Contains(_T("Header")) && fgm->MatchesMask(ext, i))
                       return ftHeader;

(Notice the "Contains").

To use this properly we should introduce constant identifiers for file groups and readable names only as an alias, which may be extendible by plugins (using a GetNewFileGroupID() method or alike).

[danselmi]

Aren't these the same extensions as used for Syntax highlighting? (configured in Settings->Editor->SyntaxHighlighting->Filemasks)

Yes, ...so what? Id that bad? ???

So I have to configure the same thing twice? Once for syntax highlighting and a second time for CC?

[darmar]

Aren't these the same extensions as used for Syntax highlighting? (configured in Settings->Editor->SyntaxHighlighting->Filemasks)

Not exactly.
It is because CodeCompletion plugin wants to make code-completion not only for C/C++, but for other languages similar to C++ too.

The logic in CC is:
if (file is C/C++ or other language but without specific code-completion plugin):
  make code-completion.

Maybe it would be possible to register language instead of file extensions.

[dmoore]

Code Select Expand


Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 8464)
@@ -727,8 +727,12 @@
             if (   ParserCommon::FileType(filename) == ParserCommon::ftOther
                 && Manager::Get()->GetPluginManager()->IsFileExtRegistered(filename) )
                 return;


The idea is the same as in dmoore's patch. The function "IsFileExtRegistered" is doing similar thing as "IsProviderFor(ed)" in this patch.

I am a little confused by this. Is it only the plugins other than CC that register files? So only IsFileExtRegistered returns true only for Fortran files if only CC and fortran plugins are installed?

For python, I thought it made the most sense to rely on the lexer to decide what to offer python code completion for, but basing it on file extensions might make the most sense in other cases. The patch I proposed leaves it to the plugin to decide what it wants to do.

[darmar]

Is it only the plugins other than CC that register files? So only IsFileExtRegistered returns true only for Fortran files if only CC and fortran plugins are installed?

I think, currently only FortranProject plugin registers the file extensions for  IsFileExtRegistered and, consequently, it returns 'true' only for Fortran files (if FortranProject plugin is active).

For python, I thought it made the most sense to rely on the lexer to decide what to offer python code completion for, but basing it on file extensions might make the most sense in other cases.

The editor's highlighting (lexer) is based on file extensions anyway.

The patch I proposed leaves it to the plugin to decide what it wants to do.

The same situation is in the current implementation: every CC plugin (FortranProject, PythonCC etc.) decides which files (recognized by file extension) it wants to handle and registers the file extensions accordingly.

I don't argue, that the current C::B implementation is better. If developers think that the new patch is better (simpler, shorter, easier to understand etc.), it is OK.

[MortenMacFly]

I think in all cases, we can summarise:
- CC can be based on either file extension or lexer type (cbEditor)
- for file extensions we have a file extension registry, that might be mis-leading
- for CC, we have an own file extension registry, that must not be compatible with the global file extension registry
- the file extension registry is not error-free, as it does not offer constant ID's for groups of files

- solution 1 (dmoore) provides an cbEditor to the plugin to have the pugin decide what to do with it
- solution 2 (darmar) provides a file extension to the plugin to have the pugin decide what to do with it and plugins can/have to register file extemsions they care for

- solution 1 is implemented via patch, not yet fully tested
- solution 2 works already

Drawbacks I see for both are (for example): STL header files. Those files do not have no file extension and no lexer style.

However, a CC plugin might know by the file's name it if can be interpreted. cbEditor provides both: The (Scintilla) editor and the file name as backup. So unless we have use cases were we have no cbEditor (do we???) this seems like a more generic interface. (One case I can think of is querying for a file's function w/o having it opened.)

Maybe a combination of both approaches is the best?

[dmoore]

Morten: agree with what you wrote above. Note that my patch is only attempting to solve the problem of what plugin should offer it's GUI when the user is editing a file. A separate issue is what files the plugin should parse to extract symbol information. E.g. for CC it is all project files and everything #included. In that case you can't rely on having an editor available. But I still think it is useful to have the option to use the language info of the editor for offering the UI if for no other reason than that the user can manually change it (this comes up a lot when editing python scripts that don't have an extension, but that could be solved with sniffing I guess)

[darmar]

I have tested dmoore's patch. It works fine with some changes.
My comments:
1) If plugin cares for code-completion list and call-tips, it should care for tool-tips too. Therefore the call to 'IsProviderFor(ed)' should be added in 'OnEditorTooltip' function too.
2) The call to 'IsProviderFor(ed)' was added in 'CodeComplete()' function. I think, this call should be moved near to the start of function 'DoCodeComplete()', because 'DoCodeComplete()' cares for making the code-completion list for the C/C++ preprocessor ('#...') and then calls 'CodeComplete()' (no other function calls 'CodeComplete()'). In my opinion, CodeCompletion plugin should suggest CC list for preprocessor for C/C++ only (e.g. '#' is start of comment in Python).

I have attached a patch for codecompletion.cpp. I have deleted calls to 'IsFileExtRegistered' in this patch also.