Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Jul 15, 2018

fixes #91

@Yaron10
Copy link

Yaron10 commented Jul 15, 2018

@ClaudiaFrank,

👍
Great!

I assume this has been a tedious work. Thank you.

@ghost
Copy link
Author

ghost commented Jul 15, 2018

@Yaron10 - finding the motivation to start is the issue mostly but once started it wasn't that painful but
for sure there are nicer, cooler, more satisfying tasks than updating the documentation :-)

CFrank added 2 commits July 16, 2018 01:34
fix for #95 - using indicator instead of style to color output
fix for #93 - using different variables for default and continue prompt
@chcg chcg added this to the v1.3 milestone Jul 17, 2018
size_t docLength = (size_t)callScintilla(SCI_GETLENGTH);
size_t realLength = length;
callScintilla(SCI_SETREADONLY, 0);
for (idx_t i = 0; i < length; ++i)
Copy link
Collaborator

@chcg chcg Jul 17, 2018

Choose a reason for hiding this comment

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

@ClaudiaFrank Why is this no longer needed?

Copy link
Author

Choose a reason for hiding this comment

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

Because the complete text should be colored - no need for special treatments like
in writeerror function but to be honest, I don't see why this special treamtent is needed at all or
what the benefit is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added with:

Revision: d725887
Author: Dave Brotherstone [email protected]
Date: 13.08.2010 00:00:27
Message:
Stripped the \r from console output

  • console.run output now a bit nicer if commands output \r\n

Modified: PythonScript/src/ConsoleDialog.cpp

Copy link
Author

Choose a reason for hiding this comment

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

but I don't see the necessity.
Example with console.write and print. Note, write mehtod does not, contrary to print,
add an eol automatically to the end of the text.

>>> console.write('hello\r')
hello
>>> console.write('hello\n')
hello
>>> console.write('hello\r\n')
hello
>>> print('hello\r')
hello
>>> print('hello\n')
hello

>>> print('hello\r\n')
hello

>>>

grafik

The different behavior of print is related to python as it is internally using \n only, that
is the reason why the 2nd and third are having an additional line - but I don't think that
this was the reason to introduce this logis at all.
Coult it be that scintilla from 2010 had problems with different EOLs?
I did a search on scintilla google groups but I haven't found one.
Today, clearly, it hasn't. I can reintroudce the code, I don't mind - Just not sure if it is needed anymore. What about keeping it for some time and if no one complains remove the logic from the
others as well?
As said, I don't mind, if I should keep it the same like the others I will do so.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems to be ok, let's see if someone finds an issue with it

//lint +e849

/* ++Autogenerated ---------------------------------------------- */
enum CaseInsensitiveBehaviour
Copy link
Collaborator

@chcg chcg Jul 17, 2018

Choose a reason for hiding this comment

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

@ClaudiaFrank Do you have an idea why this is sorted differently now? Is this expected?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this is expected because the createwrapper script
loops over a dictionary, which contrary to lists, do not
keep the ordering of its items.

I compared the old and new results and both are the same.

@chcg chcg merged commit d54a2b4 into bruderstein:master Jul 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update MENUCOMMAND list
2 participants