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

Add methods to get position from column and line in TextEdit #55102

Merged

Conversation

YuriSizov
Copy link
Contributor

@YuriSizov YuriSizov commented Nov 18, 2021

This closes godotengine/godot-proposals#1644 for master. I'm going to make another PR for 3.x, but it's going to be quite different for obvious reasons.

The added methods can be useful to draw something around specific characters. We'll have a proper API for that at some point, but this should be generally useful still. And this would allow to introduce similar methods to 3.x and preserve feature parity.


I'm really interested to see if this is a good implementation, as I don't feel comfortable working with TextServer yet and am not confident about the approach 😅

servers/text_server.cpp Outdated Show resolved Hide resolved
scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
@YuriSizov YuriSizov force-pushed the textedit-position-from-linecol branch from f14f16d to c9755a7 Compare November 19, 2021 11:39
@YuriSizov
Copy link
Contributor Author

YuriSizov commented Nov 19, 2021

I've applied suggested changes and removed some no-longer-necessary code. Retested with folding and wrapping and it seems to work as expected.

Curious about the "If the first and last character is visible, it doesn't necessarily mean all character in between are" case, but don't know what to do about it. We can remove the check for the visible range, of course. It was only proposed as an optimization step to avoid going for characters that are definitely not visible (as for the purposes of these new methods, we return (-1, -1) for those).

So please let me know what I need to do here.

Copy link
Member

@Paulb23 Paulb23 left a comment

Choose a reason for hiding this comment

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

Couple nitpicks otherwise looks good.

If the col optimisation is too much effort to work in, will be okay to leave it out.

scene/gui/text_edit.h Outdated Show resolved Hide resolved
scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
@YuriSizov YuriSizov force-pushed the textedit-position-from-linecol branch from c9755a7 to 9d0fc97 Compare November 22, 2021 12:12
@YuriSizov YuriSizov force-pushed the textedit-position-from-linecol branch from 9d0fc97 to e85e6ec Compare November 22, 2021 12:13
@YuriSizov
Copy link
Contributor Author

YuriSizov commented Nov 22, 2021

I've moved things around as requested, and added a note to the docs to explain the difference between values returned by the two methods. Since I want to keep get_pos_at_line_column, I didn't feel like renaming as suggested would work with the existing get_line_column_at_pos.

PS. See the second to last force-push for the relevant changes, the last one is just a rebase.

@akien-mga akien-mga merged commit 4e7964f into godotengine:master Nov 22, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pos2colrow and colrow2pos methods in TextEdit
4 participants