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 text input cursor setting for multi-line text input with many spaces at the line end #9319

Merged
merged 7 commits into from
Dec 12, 2024

Conversation

Districh-ru
Copy link
Collaborator

@Districh-ru Districh-ru commented Dec 5, 2024

This PR fixes the issue with setting input cursor position when there are many spaces in multi-line text input: #9281 (comment)

Now the multi-line text input field acts like in the original editor - is does not cut spaces at the line end to show user that there are many spaces there and to allow position the cursor at any place of such consequent spaces.

This PR also:

  • moves getTextInputCursorPosition() to ui_tool;
  • fixes horizontal position of the multi-font text line separated by -
    master build:
    bug
    this PR:
    nobug

@Districh-ru Districh-ru added bug Something doesn't work ui UI/GUI related stuff labels Dec 5, 2024
@Districh-ru Districh-ru added this to the 1.1.5 milestone Dec 5, 2024
@Districh-ru Districh-ru self-assigned this Dec 5, 2024
@Districh-ru
Copy link
Collaborator Author

Hi, @ihhub!
I've faced a strange code style check behavior: https://github.com/ihhub/fheroes2/actions/runs/12183834934/job/33986233552?pr=9319
The code was formatted using our .clang-format file.
And also the suggested change (ed15349) looks not good to me.

@oleg-derevenetz
Copy link
Collaborator

I've faced a strange code style check behavior: https://github.com/ihhub/fheroes2/actions/runs/12183834934/job/33986233552?pr=9319 The code was formatted using our .clang-format file. And also the suggested change (ed15349) looks not good to me.

This formatting is because clang-format treats this as an assignment operation, and there is no knob yet to format assignment operations and pure virtual function declarations in a different way. Please see llvm/llvm-project#57569 for details.

@Districh-ru
Copy link
Collaborator Author

Thanks for clarifying, @oleg-derevenetz!
Visual Studio 2019 has clang-format v.12.0 and uses it even if a newer version installed in the system.
I've updated this clang-format to the latest version (making a symlink to LLVM folder, VS does not look in the PATH environment variable folders) and now formatting goes like Code Style Check wants.

@oleg-derevenetz
Copy link
Collaborator

Hi @Districh-ru

I've updated this clang-format to the latest version (making a symlink to LLVM folder, VS does not look in the PATH environment variable folders) and now formatting goes like Code Style Check wants.

There is no need to create symlinks, MSVC can use a custom clang-format executable:

clang-format

Copy link
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

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

Hi @Districh-ru , I left 2 comments here. Can you please take a look at them?

src/fheroes2/gui/ui_text.h Outdated Show resolved Hide resolved
src/fheroes2/gui/ui_text.cpp Outdated Show resolved Hide resolved
@Districh-ru Districh-ru requested a review from ihhub December 8, 2024 15:53
Copy link
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

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

Hi @Districh-ru , I took another round of review of this pull request as @Branikolog hasn't reviewed yet. I left few suggestions for improvements. Could you please take a look at them?

src/fheroes2/gui/ui_text.h Outdated Show resolved Hide resolved
src/fheroes2/gui/ui_text.h Outdated Show resolved Hide resolved
src/fheroes2/gui/ui_text.h Outdated Show resolved Hide resolved
src/fheroes2/gui/ui_text.h Outdated Show resolved Hide resolved
@ihhub ihhub merged commit 1265680 into ihhub:master Dec 12, 2024
20 checks passed
@ihhub
Copy link
Owner

ihhub commented Dec 12, 2024

@Districh-ru , I made the tests myself. Everything works perfect. Thank you!

@Districh-ru Districh-ru deleted the fix-input-cursor-position-setting branch December 13, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something doesn't work ui UI/GUI related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants