Skip to content
This repository was archived by the owner on Feb 20, 2024. It is now read-only.

Fix/ticket/832/1 #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix/ticket/832/1 #13

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31886a3
*SDK: Add functions for detecting line ending and identation characte…
bluehazzard Apr 27, 2020
7466de4
*wxSmith: Add correct handling of line endings and identation charact…
bluehazzard Apr 27, 2020
8360a5c
*scriptedwizard: Update all wxSmith scripts use use tab character and…
bluehazzard Apr 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/include/globals.h
View file
Original file line number Diff line number Diff line change
@@ -260,12 +260,28 @@ extern DLLIMPORT bool cbResolveSymLinkedDirPath(wxString& dirpath);
/// @return The resolved path or the same path if not a symlink.
extern DLLIMPORT wxString cbResolveSymLinkedDirPathRecursive(wxString dirpath);

/**
* Return true if the indentation line endings should be consistent in the file,
* so you have to detect the line ending of the file you are editing.
* If the value is false use cbGetEOLStr(-1) go get the line ending.
*/
extern DLLIMPORT bool cbEnsureLineEndingConsistency();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These names are wrong. There return a config value. Naming them like this implies they do some action. Name them as getters. And either all of them should have cb prefix or non of them. I think we should start prefixing them.


/** Reads settings if eolMode is -1
* Expected input (defined in sdk/wxscintilla/include/wx/wxscintilla.h) is:
* wxSCI_EOL_CRLF=0, wxSCI_EOL_CR=1, or wxSCI_EOL_LF=2
*/
extern DLLIMPORT wxString GetEOLStr(int eolMode = -1);

/**
* Return true if the indentation style should be detected automatically, or false if the indentation should be used like in the settings
*/
extern DLLIMPORT bool cbDetectTabStrAutomatically();
/**
* Return the tab character as set in the settings. (\t or the set number of spaces)
*/
extern DLLIMPORT wxString cbGetTabStr();

extern DLLIMPORT wxString URLEncode(const wxString &str);

extern DLLIMPORT wxString ExpandBackticks(wxString &str);
111 changes: 61 additions & 50 deletions src/plugins/contrib/wxSmith/wxscoder.cpp
View file
Original file line number Diff line number Diff line change
@@ -271,19 +271,17 @@ void wxsCoder::FlushFile(const wxString& FileName)

if ( Editor )
{
wxString EOL;
while ( Changes )
{
CodeChange* Next = Changes->Next;
ApplyChangesEditor(Editor,Changes->Header,Changes->End,Changes->Code,Changes->CodeHasHeader,Changes->CodeHasEnd,EOL);
ApplyChangesEditor(Editor,Changes->Header,Changes->End,Changes->Code,Changes->CodeHasHeader,Changes->CodeHasEnd);
delete Changes;
Changes = Next;
}
}
else
{
// Reading file content
wxString EOL;
bool HasChanged = false;

//wxStopWatch SW;
@@ -299,7 +297,7 @@ void wxsCoder::FlushFile(const wxString& FileName)
while ( Changes )
{
CodeChange* Next = Changes->Next;
ApplyChangesString(Content,Changes->Header,Changes->End,Changes->Code,Changes->CodeHasHeader,Changes->CodeHasEnd,HasChanged,EOL);
ApplyChangesString(Content,Changes->Header,Changes->End,Changes->Code,Changes->CodeHasHeader,Changes->CodeHasEnd,HasChanged);
delete Changes;
Changes = Next;
}
@@ -325,32 +323,65 @@ void wxsCoder::FlushFile(const wxString& FileName)
CodeChanges[Index] = 0;
}

bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const wxString& End,wxString& Code,bool CodeHasHeader,bool CodeHasEnd,wxString& EOL)
void wxsCoder::GetLineEndingIndentation(const wxString& line, wxString& indentation, wxString& lineending)
{
cbStyledTextCtrl* Ctrl = Editor->GetControl();
int FullLength = Ctrl->GetLength();
const size_t length = line.length();
// Fetching indentation
if(cbDetectTabStrAutomatically() && !line.IsEmpty() )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please spend some time to be consistent with formatting. Your formatting is always random.
Stick to something like this:

if (condition) // space before the bracket and no spaces around the condition.
{
    <somehting>
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you calling IsEmpty here? You already have the length. Also it is preferable to use the stl versions.

{
wxString BaseIndentation;

if ( EOL.IsEmpty() )
size_t IndentPos = length;
while ( --IndentPos >= 0 )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope you know that size_t is an unsigned type, which means that counting backwards is really dangerous.

I'm pretty sure this loop has a bug in it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you just use one of the rfind methods provided by wxString?

{
wxChar ch = line.GetChar(IndentPos);
if ( (ch == _T('\n')) || (ch == _T('\r')) ) break;
}
while ( ++IndentPos < length )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this expected to start with IndentPos=0? I don't understand this loop. What does it do?

{
wxChar ch = line.GetChar(IndentPos);
BaseIndentation.Append(
( ch == _T('\t') ) ? _T('\t') : _T(' '));
}

indentation = BaseIndentation;
}
else {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brace on the wrong line...

indentation = cbGetTabStr();
}

if ( cbEnsureLineEndingConsistency() && !line.IsEmpty() )
{
// Detecting EOL style in source
for ( int i=0; i<FullLength; i++ )
for ( int i=0; i<length; i++ )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you iterating the whole line to detect what is the EOL?

{
wxChar ch = Ctrl->GetCharAt(i);
wxChar ch = line.GetChar(i);
if ( ch==_T('\n') || ch==_T('\r') )
{
EOL = ch;
if ( ++i < FullLength )
lineending = ch;
if ( ++i < length )
{
wxChar ch2 = Ctrl->GetCharAt(i);
wxChar ch2 = line.GetChar(i);
if ( (ch2==_T('\n') || ch2==_T('\r')) && ch!=ch2 )
{
EOL.Append(ch2);
lineending.Append(ch2);
}
}
break;
}
}
}
else
{
lineending = GetEOLStr(-1);
}
}


bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const wxString& End,wxString& Code,bool CodeHasHeader,bool CodeHasEnd)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is also wrong. It should be something like ApplyChangesToEditor

{
cbStyledTextCtrl* Ctrl = Editor->GetControl();
int FullLength = Ctrl->GetLength();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const?


// Searching for beginning of section to replace
Ctrl->SetSearchFlags(wxSCI_FIND_MATCHCASE);
@@ -366,6 +397,10 @@ bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const
return false;
}

wxString EOL;
wxString tab;
GetLineEndingIndentation(Ctrl->GetLine(Ctrl->LineFromPosition(Position)) , tab, EOL);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you have to copy the line to a string? Can't you work using the wxScintilla API to do the same thing?


// Beginning of this code block is in Position, now searching for end
Ctrl->SetTargetStart(Position);
Ctrl->SetTargetEnd(FullLength);
@@ -378,7 +413,6 @@ bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const

return false;
}

// Fetching indentation
wxString BaseIndentation;
int IndentPos = Position;
@@ -393,8 +427,7 @@ bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const
BaseIndentation.Append(
( ch == _T('\t') ) ? _T('\t') : _T(' '));
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is duplicated, but here it uses a signed type, so it is actually correct, whatever it does. It is not clear.


Code = RebuildCode(BaseIndentation,Code.c_str(),(int)Code.Length(),EOL);
Code = RebuildCode(BaseIndentation, Code.c_str(),(int)Code.Length(), EOL, tab);

// Fixing up positions to contain or not header / ending sequence
if ( !CodeHasHeader ) Position += Header.Length();
@@ -423,32 +456,11 @@ bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const
return true;
}

bool wxsCoder::ApplyChangesString(wxString& BaseContent,const wxString& Header,const wxString& End,wxString& Code,bool CodeHasHeader,bool CodeHasEnd,bool& HasChanged,wxString& EOL)
bool wxsCoder::ApplyChangesString(wxString& BaseContent,const wxString& Header,const wxString& End,wxString& Code,bool CodeHasHeader,bool CodeHasEnd,bool& HasChanged)
{
wxString Content = BaseContent;
if ( EOL.IsEmpty() )
{
// Detecting EOL in this sources
for ( size_t i=0; i<Content.Length(); i++ )
{
wxChar ch = Content.GetChar(i);
if ( ch==_T('\n') || ch==_T('\r') )
{
EOL = ch;
if ( ++i < Content.Length() )
{
wxChar ch2 = Content.GetChar(i);
if ( (ch2==_T('\n') || ch2==_T('\r')) && ch!=ch2 )
{
EOL.Append(ch2);
}
}
break;
}
}
}

// Search for header
// Search for header
int Position = Content.First(Header);

if ( Position == -1 )
@@ -457,6 +469,11 @@ bool wxsCoder::ApplyChangesString(wxString& BaseContent,const wxString& Header,c
return false;
}

wxString EOL;
wxString tab;
int start = BaseContent.rfind('\n', Position);
GetLineEndingIndentation(BaseContent.substr(start < 0 ? 0 : start) , tab, EOL);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment what is this doing. Why do you need the substr? Why aren't you comparing with wxString::npos?


// Skipping header if necessary
int IndentPos = Position;
int IndentMax = Position;
@@ -489,7 +506,7 @@ bool wxsCoder::ApplyChangesString(wxString& BaseContent,const wxString& Header,c
( ch == _T('\t') ) ? _T('\t') : _T(' '));
}

Code = RebuildCode(BaseIndentation,Code.c_str(),Code.Length(),EOL);
Code = RebuildCode(BaseIndentation,Code.c_str(), Code.Length(), EOL, tab);

// Checking if code has really changed
if ( Content.Mid(0,EndPosition) == Code )
@@ -505,15 +522,9 @@ bool wxsCoder::ApplyChangesString(wxString& BaseContent,const wxString& Header,c
return true;
}

wxString wxsCoder::RebuildCode(wxString& BaseIndentation,const wxChar* Code,int CodeLen,wxString& EOL)
wxString wxsCoder::RebuildCode(wxString& BaseIndentation, const wxChar* Code, int CodeLen, wxString EOL, const wxString& tab)
{
wxString Tab;
bool UseTab = Manager::Get()->GetConfigManager(_T("editor"))->ReadBool(_T("/use_tab"), false);
int TabSize = Manager::Get()->GetConfigManager(_T("editor"))->ReadInt(_T("/tab_size"), 4);
if ( !UseTab )
{
Tab.Append(_T(' '),TabSize);
}
const bool UseTab = tab == _("\t");

if ( EOL.IsEmpty() )
EOL = GetEOLStr();
@@ -535,7 +546,7 @@ wxString wxsCoder::RebuildCode(wxString& BaseIndentation,const wxChar* Code,int
Result << BaseIndentation;
break;
}
case _T('\t'): if ( UseTab ) { Result << Tab; break; }
case _T('\t'): if ( !UseTab ) { Result << tab; break; }

default: Result << *Code;
}
15 changes: 8 additions & 7 deletions src/plugins/contrib/wxSmith/wxscoder.h
View file
Original file line number Diff line number Diff line change
@@ -91,6 +91,12 @@ class wxsCoder: public wxEvtHandler
/** \brief Function getting singleton object from system */
static wxsCoder* Get() { return Singleton; }

/** \brief detect the tab style (indentation) and line ending of the line */
static void GetLineEndingIndentation(const wxString& line, wxString& indentation, wxString& lineending);

/** \brief Rebuilding code to support current editor settings */
static wxString RebuildCode(wxString& BaseIndentation, const wxChar* Code, int CodeLen, wxString EOL, const wxString& tab);

private:

/** \brief Structure which contains one data change */
@@ -124,8 +130,7 @@ class wxsCoder: public wxEvtHandler
const wxString& End,
wxString& Code,
bool CodeHasHeader,
bool CodeHasEnd,
wxString& EOL);
bool CodeHasEnd);

/** \brief Applying changes to string (file's content) */
bool ApplyChangesString(
@@ -135,8 +140,7 @@ class wxsCoder: public wxEvtHandler
wxString& Code,
bool CodeHasHeader,
bool CodeHasEnd,
bool& HasChanged,
wxString& EOL);
bool& HasChanged);

/** \brief Flushing all changes for given file */
void FlushFile(const wxString& FileName);
@@ -147,9 +151,6 @@ class wxsCoder: public wxEvtHandler
/** \brief Flush timer procedure */
void FlushTimerEvent(wxTimerEvent& event);

/** \brief Rebuilding code to support current editor settings */
wxString RebuildCode(wxString& BaseIndentation,const wxChar* Code,int CodeLen,wxString& EOL);

/** \brief Cutting off given number of spaces at every new line */
wxString CutSpaces(wxString Code,int Count);

15 changes: 9 additions & 6 deletions src/plugins/contrib/wxSmith/wxwidgets/wxseventseditor.cpp
View file
Original file line number Diff line number Diff line change
@@ -429,17 +429,20 @@ bool wxsEventsEditor::CreateNewFunction(const wxsEventDesc* Event,const wxString
return false;
}

cbStyledTextCtrl* Ctrl = Editor->GetControl();
int LineNumber = Ctrl->GetLineCount();
wxString eof, tab;
wxsCoder::GetLineEndingIndentation(Ctrl->GetLine(LineNumber/2), tab, eof); // Get line endings form a random position in the editor control
wxString NewFunctionCode;
NewFunctionCode <<
_T("\n")
_T("void ") << m_Class << _T("::") << NewFunctionName << _T("(") << Event->ArgType << _T("& event)\n")
_T("{\n")
_T("}\n");
eof <<
_T("void ") << m_Class << _T("::") << NewFunctionName << _T("(") << Event->ArgType << _T("& event)") << eof <<
_T("{") << eof <<
_T("}") << eof;

// TODO: Replace line endings with propert string

cbStyledTextCtrl* Ctrl = Editor->GetControl();
int LineNumber = Ctrl->GetLineCount();

Ctrl->DocumentEnd();
Ctrl->AddText(NewFunctionCode);
Editor->SetModified();
11 changes: 6 additions & 5 deletions src/plugins/contrib/wxSmith/wxwidgets/wxsitemres.cpp
View file
Original file line number Diff line number Diff line change
@@ -375,9 +375,9 @@ bool wxsItemRes::CreateNewResource(NewResourceParams& Params)
}
}
Header.Replace(_T("$(HandlersScope)"),Scope);

// TODO: Use wxsCoder to save file's content, so it will
// have proper encoding and EOL stuff
wxString Indentation, EOL, tab;
wxsCoder::GetLineEndingIndentation(wxEmptyString, tab, EOL);
Header = wxsCoder::RebuildCode(Indentation, Header.c_str(), Header.length(), EOL, tab);
if ( !HdrFile.Write(Header) ) return false;
}

@@ -422,8 +422,9 @@ bool wxsItemRes::CreateNewResource(NewResourceParams& Params)
SourceStr.Replace(_T("$(BaseClassName)"),Params.BaseClass);
SourceStr.Replace(_T("$(CtorInit)"),CtorInitCode);
SourceStr.Replace(_T("$(InternalHeadersPch)"),IntHeadersPch);
// TODO: Use wxsCoder to save file's content, so it will
// have proper encoding and EOL stuff
wxString Indentation, EOL, tab;
wxsCoder::GetLineEndingIndentation(wxEmptyString, tab, EOL);
SourceStr = wxsCoder::RebuildCode(Indentation, SourceStr.c_str(), SourceStr.length(), EOL, tab);
if ( !SrcFile.Write(SourceStr) ) return false;
}

14 changes: 7 additions & 7 deletions src/plugins/scriptedwizard/resources/wxwidgets/common/app.cpp
View file
Original file line number Diff line number Diff line change
@@ -22,11 +22,11 @@ IMPLEMENT_APP([CLASS_PREFIX]App);

bool [CLASS_PREFIX]App::OnInit()
{
[IF WXFRAME][CLASS_PREFIX]Frame* frame = new [CLASS_PREFIX]Frame(0L[IF NONE], _("wxWidgets Application Template")[ENDIF NONE]);
[IF WINDOWS]frame->SetIcon(wxICON(aaaa)); // To Set App Icon[ENDIF WINDOWS]
frame->Show();[ENDIF WXFRAME]
[IF WXDIALOG][CLASS_PREFIX]Dialog* dlg = new [CLASS_PREFIX]Dialog(0L[IF NONE], _("wxWidgets Application Template")[ENDIF NONE]);
[IF WINDOWS]dlg->SetIcon(wxICON(aaaa)); // To Set App Icon[ENDIF WINDOWS]
dlg->Show();[ENDIF WXDIALOG]
return true;
[IF WXFRAME][CLASS_PREFIX]Frame* frame = new [CLASS_PREFIX]Frame(0L[IF NONE], _("wxWidgets Application Template")[ENDIF NONE]);
[IF WINDOWS]frame->SetIcon(wxICON(aaaa)); // To Set App Icon[ENDIF WINDOWS]
frame->Show();[ENDIF WXFRAME]
[IF WXDIALOG][CLASS_PREFIX]Dialog* dlg = new [CLASS_PREFIX]Dialog(0L[IF NONE], _("wxWidgets Application Template")[ENDIF NONE]);
[IF WINDOWS]dlg->SetIcon(wxICON(aaaa)); // To Set App Icon[ENDIF WINDOWS]
dlg->Show();[ENDIF WXDIALOG]
return true;
}
Loading
Oops, something went wrong.