Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Unicode Problem. Issue Name 'On Debug Build, Korean Input has As… #4530

Closed
wants to merge 3 commits into from

Conversation

ffdd270
Copy link

@ffdd270 ffdd270 commented May 26, 2018

When compiled in a Unicode environment, the isAllDigits function uses the iswdigit function.
Other environments use the isdigit function.

Fix Issue #4529.

…ert.'

When compiled in a Unicode environment, the isAllDigits function uses the iswdigit function.
Other environments use the isdigigit function.
@@ -43,7 +43,11 @@ static bool isInList(generic_string word, const vector<generic_string> & wordArr

static bool isAllDigits(const generic_string &str)
{
#ifdef _UNICODE
Copy link
Contributor

Choose a reason for hiding this comment

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

Notepad++ is Unicode. ANSI version was dropped long back. Moreover, generic_string is typedef std::basic_string<TCHAR> generic_string; where TCHAR is WCHAR because of Unicode project setting. Also refer discussion - #3198 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

Then, Can I erase the compiler encounters without considering ASCII situations? And was there an issue in the context of using iswdigit?

Copy link

Choose a reason for hiding this comment

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

Notepad++ is Unicode. ANSI version was dropped long back. Moreover, generic_string is typedef std::basic_string<TCHAR> generic_string; where TCHAR is WCHAR because of Unicode project setting. Also refer discussion - #3198 (comment)

Then why not replace all references of T in the entire project with W. Add some static_assert or #define all T-names to something that would cause a compiler error, and preventing T-uses.

@@ -43,11 +43,7 @@ static bool isInList(generic_string word, const vector<generic_string> & wordArr

static bool isAllDigits(const generic_string &str)
{
#ifdef _UNICODE
return std::all_of(str.begin(), str.end(), ::iswdigit);
Copy link
Contributor

Choose a reason for hiding this comment

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

_istdigit if you want to care about both. Since generic_string is typedef'd, I'd advice using a TCHAR version.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for Code Review. I will fix that on next commit.

but I have one question, generic_string is templated string? For Example. std::wstring same as generic_string?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. From Common.h:

typedef std::basic_string<TCHAR> generic_string;
typedef std::basic_stringstream<TCHAR> generic_stringstream;

Copy link
Author

Choose a reason for hiding this comment

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

Thank You For Answer. I fix that.

@chcg
Copy link
Contributor

chcg commented Oct 31, 2018

There a some further code areas which use:

isdigit

instead of

_istdigit

on generic_string, e.g.
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/master/PowerEditor/src/WinControls/PluginsAdmin/pluginsAdmin.h#L54
or TCHAR:

for (*length = 0; isdigit(*p); ++(*length))

and maybe also the scintilla component need to be checked.

@CookiePLMonster
Copy link
Contributor

and maybe also the scintilla component need to be checked.

Perhaps - but what's the convention on making changes inside scintilla? It's not exactly clear right now.

@chcg chcg added the bug label Dec 12, 2018
@zufuliu
Copy link
Contributor

zufuliu commented May 26, 2019

Scintilla (except lexers, which mostly maintained by external developers) doesn't directly call ctype functions.

https://sourceforge.net/p/scintilla/code/ci/348e55f8107caddf4f48b8c514caa90f1c886c48/

SinghRajenM referenced this pull request Jun 11, 2019
Fix debug mode assert in `AutoCompletion::isAllDigits` methode while inputting non ASCII characters.

Fix #5280
@donho donho closed this in 105dceb Jun 25, 2019
kspalaiologos pushed a commit to kspalaiologos/notepad-plus-plus that referenced this pull request Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants