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 scrolling of UTF-8 text. #384

Merged
merged 2 commits into from Dec 7, 2017

Conversation

Projects
None yet
4 participants
@AlexeyBond
Contributor

AlexeyBond commented Apr 7, 2017

Fix issue #129 (UPD: should fix #223 and #45 too).

Port of PR #292 to master with fixes for new scroll modes.

  • right scroll now works with color changes
@Erinor2

This comment has been minimized.

Show comment
Hide comment
@Erinor2

Erinor2 Apr 7, 2017

Contributor

Hello Alexey,

Will #377 also be fixed?

Regards,
Philippe

Contributor

Erinor2 commented Apr 7, 2017

Hello Alexey,

Will #377 also be fixed?

Regards,
Philippe

@ght

I'm not that experienced with C++, but I found some bugs in your PR that might cause out-of-bounds array access if the text contains invalid UTF-8 byte sequences.

There is a patch available on the old SourceForge issue tracker (https://sourceforge.net/p/conky/bugs/341/#73c2) that solves exactly this bug and it looks cleaner that this PR, maybe that should be adapted instead.

@@ -298,6 +298,12 @@ void generate_text_internal(char *, int, struct text_object);
int percent_print(char *, int, unsigned);
void human_readable(long long, char *, int);
#ifdef BUILD_X11

This comment has been minimized.

@ght

ght Jun 5, 2017

scroll can be used with out_to_console, so the #ifdef BUILD_X11 should probably removed.

@ght

ght Jun 5, 2017

scroll can be used with out_to_console, so the #ifdef BUILD_X11 should probably removed.

This comment has been minimized.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

Variable utf8_mode is defined inside of #ifdef BUILD_X11 ... #endif in conky.cc, so if it will be used when BUILD_X11 is not defined then the program won't link.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

Variable utf8_mode is defined inside of #ifdef BUILD_X11 ... #endif in conky.cc, so if it will be used when BUILD_X11 is not defined then the program won't link.

This comment has been minimized.

@ght

ght Jun 6, 2017

My bad, didn't notice that.

@ght

ght Jun 6, 2017

My bad, didn't notice that.

* @param c first byte of the character
*/
inline int scroll_character_length(char c) {
#ifdef BUILD_X11

This comment has been minimized.

@ght

ght Jun 5, 2017

(see above comment)

@ght

ght Jun 5, 2017

(see above comment)

This comment has been minimized.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see answer to comment above)

Scroll still will work in console mode, but without UTF-8 support.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see answer to comment above)

Scroll still will work in console mode, but without UTF-8 support.

Show outdated Hide outdated src/scroll.cc Outdated
* Check if a byte should be skipped when counting characters to scroll text to right.
*/
inline bool scroll_check_skip_byte(char c) {
#ifdef BUILD_X11

This comment has been minimized.

@ght

ght Jun 5, 2017

(see above comment)

@ght

ght Jun 5, 2017

(see above comment)

This comment has been minimized.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see responses to comments above)

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see responses to comments above)

Show outdated Hide outdated src/scroll.cc Outdated
Show outdated Hide outdated src/scroll.cc Outdated
Show outdated Hide outdated src/scroll.cc Outdated
@AlexeyBond

Differences from that patch were described in my PR fixing the same bug for older version of conky (#292). This P/R includes also fixes for new features (additional scroll modes) and does not contain bugs of aidecoe's patch (unstable length of scroll field, interpretation of characters as UTF-8 in non-UTF-8 mode).

@@ -298,6 +298,12 @@ void generate_text_internal(char *, int, struct text_object);
int percent_print(char *, int, unsigned);
void human_readable(long long, char *, int);
#ifdef BUILD_X11

This comment has been minimized.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

Variable utf8_mode is defined inside of #ifdef BUILD_X11 ... #endif in conky.cc, so if it will be used when BUILD_X11 is not defined then the program won't link.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

Variable utf8_mode is defined inside of #ifdef BUILD_X11 ... #endif in conky.cc, so if it will be used when BUILD_X11 is not defined then the program won't link.

* @param c first byte of the character
*/
inline int scroll_character_length(char c) {
#ifdef BUILD_X11

This comment has been minimized.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see answer to comment above)

Scroll still will work in console mode, but without UTF-8 support.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see answer to comment above)

Scroll still will work in console mode, but without UTF-8 support.

* Check if a byte should be skipped when counting characters to scroll text to right.
*/
inline bool scroll_check_skip_byte(char c) {
#ifdef BUILD_X11

This comment has been minimized.

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see responses to comments above)

@AlexeyBond

AlexeyBond Jun 5, 2017

Contributor

(see responses to comments above)

Show outdated Hide outdated src/scroll.cc Outdated
Show outdated Hide outdated src/scroll.cc Outdated
Show outdated Hide outdated src/scroll.cc Outdated
Show outdated Hide outdated src/scroll.cc Outdated

@brndnmtthws brndnmtthws merged commit cbe403b into brndnmtthws:master Dec 7, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment