Question about rev 9399

[ollydbg]

Hi, alpha, I see rev 9399, I have some comments on this:
In rev 8332, I introduce the "auto" mode for automatically detect the eol mode by scanning the files.
Which I have:

Code

@@ -1575,6 +1665,14 @@ void cbEditor::InternalSetEditorStyleAfterFileOpen(cbStyledTextCtrl* control)
 
     ConfigManager* mgr = Manager::Get()->GetConfigManager(_T("editor"));
 
+    // set the EOL, fall back value: Windows takes CR+LF, other platforms LF only
+    int eolMode = mgr->ReadInt(_T("/eol/eolmode"), platform::windows ? wxSCI_EOL_CRLF : wxSCI_EOL_LF);
+
+    if (eolMode == 3) //auto detect the EOL
+        eolMode = DetectLineEnds(control);
+
+    control->SetEOLMode(eolMode);
+
     // Interpret #if/#else/#endif to grey out code that is not active
     control->SetProperty(_T("lexer.cpp.track.preprocessor"), mgr->ReadBool(_T("/track_preprocessor"), true) ? _T("1") : _T("0"));

When an editor is opened, I detect the eol mode, and set this mode by Control->SetEOLMode().

Now, what you change in the code looks like "use the system default eol if the setting is auto", see:

Code

 src/plugins/contrib/wxSmith/wxscoder.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/plugins/contrib/wxSmith/wxscoder.cpp b/src/plugins/contrib/wxSmith/wxscoder.cpp
index 313929e..8a68f18 100644
--- a/src/plugins/contrib/wxSmith/wxscoder.cpp
+++ b/src/plugins/contrib/wxSmith/wxscoder.cpp
@@ -517,7 +517,10 @@ wxString wxsCoder::RebuildCode(wxString& BaseIndentation,const wxChar* Code,int
 
     if ( EOL.IsEmpty() )
     {
-        int EolMode = Manager::Get()->GetConfigManager(_T("editor"))->ReadInt(_T("/eol/eolmode"), 0);
+        static const int defEol = platform::windows ? wxSCI_EOL_CRLF : wxSCI_EOL_LF;
+        int EolMode = Manager::Get()->GetConfigManager(_T("editor"))->ReadInt(_T("/eol/eolmode"), defEol);
+        if (EolMode == 3) // auto-detect EOL
+            EolMode = defEol;
         switch ( EolMode )
         {
             case 1:  EOL = _T("\r"); break;

So the suggest way is:
If the option is "auto" in the settings, and the file is already opened in the editor, use the eol of the control directly, because the DetectLineEnds(control) function is already run, the eol is correctly set for the current control.

If the file is not opened, and I think you should use the similar way of DetectLineEnds(control), then set the correct eol mode for the generated code.

If the file is empty (like the one going to create a new file), thus we can use system default eol for "auto" mode.

The reason is that I can have many LF source files under Windows system, in this case, I would like wxsmith to generate code in LF mode, this avoid a mixed eol issue.

[Alpha]

The two functions that call wxsCoder::RebuildCode() (ApplyChangesEditor() and ApplyChangesString()) both run detection on their given context, and pass the detected EOL to RebuildCode().  The system default setting is only used if the previous detection has failed.
Class and scripted wizards do not have any obvious context to try to guess the desired EOL style from, so I set them to handle "auto" with system default.

Is there a better way to handle these cases?

[ollydbg]

The two functions that call wxsCoder::RebuildCode() (ApplyChangesEditor() and ApplyChangesString()) both run detection on their given context, and pass the detected EOL to RebuildCode().  The system default setting is only used if the previous detection has failed.
Class and scripted wizards do not have any obvious context to try to guess the desired EOL style from, so I set them to handle "auto" with system default.

Ok, thanks for the explanation.

Is there a better way to handle these cases?

I can't find a better way than what you did.

[oBFusCATed]

Alpha: Could you wrap this code in a function? It seems the code is similar at the places you've changed.

[Alpha]

Alpha: Could you wrap this code in a function?

Okay.  I will add it as a global (unless you think it should belong to cbEditor or something else instead?).

[oBFusCATed]

I'm not sure where is a good place to put this function. So do what you think it is best.

[Alpha]

Attached patch adds this function.  I am not thinking very clearly right now, so I will leave this here for a bit before committing it.

[ollydbg]

Attached patch adds this function.  I am not thinking very clearly right now, so I will leave this here for a bit before committing it.

Good, Maybe "GetEolStr" should be "GetEOLStr"?

EDIT: We have already functions like "SetEOLMode".

[Alpha]

Renamed and committed.

[oBFusCATed]

Please use enum instead of int for the parameter of "wxString GetEOLStr(int eolMode)".

[Alpha]

I used int because it was defined as:

Code: sdk/wxscintilla/include/wx/wxscintilla.h:79

#define wxSCI_EOL_CRLF 0
#define wxSCI_EOL_CR 1
#define wxSCI_EOL_LF 2

Should I create an enum with these values?

[oBFusCATed]

Then probably it is better to add documentation for the expected values...