Fix/ticket/832/1 #13
Fix/ticket/832/1 #13
Conversation
…rs. Add also script binding for this functions
…er in wxSmith code. (fix #832, thanks Miguel Gimenez)
… correct line endings, so the code can replace them accordingly
| * 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(); |
There was a problem hiding this comment.
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.
| } | ||
|
|
||
|
|
||
| bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const wxString& End,wxString& Code,bool CodeHasHeader,bool CodeHasEnd) |
There was a problem hiding this comment.
This name is also wrong. It should be something like ApplyChangesToEditor
| bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const wxString& End,wxString& Code,bool CodeHasHeader,bool CodeHasEnd) | ||
| { | ||
| cbStyledTextCtrl* Ctrl = Editor->GetControl(); | ||
| int FullLength = Ctrl->GetLength(); |
| int FullLength = Ctrl->GetLength(); | ||
| const size_t length = line.length(); | ||
| // Fetching indentation | ||
| if(cbDetectTabStrAutomatically() && !line.IsEmpty() ) |
There was a problem hiding this comment.
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>
}
| int FullLength = Ctrl->GetLength(); | ||
| const size_t length = line.length(); | ||
| // Fetching indentation | ||
| if(cbDetectTabStrAutomatically() && !line.IsEmpty() ) |
There was a problem hiding this comment.
Why are you calling IsEmpty here? You already have the length. Also it is preferable to use the stl versions.
| wxChar ch = line.GetChar(IndentPos); | ||
| if ( (ch == _T('\n')) || (ch == _T('\r')) ) break; | ||
| } | ||
| while ( ++IndentPos < length ) |
There was a problem hiding this comment.
Is this expected to start with IndentPos=0? I don't understand this loop. What does it do?
| { | ||
| // Detecting EOL style in source | ||
| for ( int i=0; i<FullLength; i++ ) | ||
| for ( int i=0; i<length; i++ ) |
There was a problem hiding this comment.
Why are you iterating the whole line to detect what is the EOL?
| @@ -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); | |||
There was a problem hiding this comment.
Why do you have to copy the line to a string? Can't you work using the wxScintilla API to do the same thing?
| @@ -393,8 +427,7 @@ bool wxsCoder::ApplyChangesEditor(cbEditor* Editor,const wxString& Header,const | |||
| BaseIndentation.Append( | |||
| ( ch == _T('\t') ) ? _T('\t') : _T(' ')); | |||
| } | |||
There was a problem hiding this comment.
This code is duplicated, but here it uses a signed type, so it is actually correct, whatever it does. It is not clear.
| wxString EOL; | ||
| wxString tab; | ||
| int start = BaseContent.rfind('\n', Position); | ||
| GetLineEndingIndentation(BaseContent.substr(start < 0 ? 0 : start) , tab, EOL); |
There was a problem hiding this comment.
Add comment what is this doing. Why do you need the substr? Why aren't you comparing with wxString::npos?
Add detection and right usage of line endings and indentation to wxSmith
As mentioned in chat, here is a pull request for better commenting and discussing