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 TextEdit::get_line_count() when lines are wrapped #39191

Closed
wants to merge 1 commit into from

Conversation

kopcion
Copy link

@kopcion kopcion commented May 31, 2020

After this PR TextEdit::get_line_count() will return the number of entries in text vector, as well as a number of times those lines wrap around so that the result is the same as what users can see on screen, as opposed to a number of '\n' characters.

Bugsquad edit: Fixes #38180.

@YeldhamDev YeldhamDev added bug topic:gui cherrypick:3.x Considered for cherry-picking into a future 3.x release labels May 31, 2020
@YeldhamDev YeldhamDev added this to the 4.0 milestone May 31, 2020
@YeldhamDev
Copy link
Member

Please use a more descriptive commit message.

	TextEdit::get_line_count() now considers visible lines instead of lines
	in source text before wrapping by adding for each line
	TextEdit::times_line_wraps(int).
@kopcion
Copy link
Author

kopcion commented May 31, 2020

I hope this commit message is more descriptive :)

@akien-mga akien-mga changed the title Fix to issue #38180 Fix TextEdit::get_line_count() when lines are wrapped Jun 3, 2020
@akien-mga
Copy link
Member

akien-mga commented Jun 3, 2020

To clarify about the commit message, we usually favor having a commit title that accurately describe what the fixed bug is, instead of only pointing to a GitHub issue number (which can't be read through git log). See https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind

So here something like this would be great:

Fix TextEdit::get_line_count() when lines are wrapped

TextEdit::get_line_count() now considers visible lines instead of lines in source
text before wrapping by adding for each line TextEdit::times_line_wraps(int).

Fixes #38180.

Using "Fixes #issue" in the commit description / PR description also means that GitHub will automatically close the issue when the PR is merged: https://help.github.com/en/github/managing-your-work-on-github/closing-issues-using-keywords

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.

This is more of a enhancement then bug, get_line_count is used for looping over the "raw" lines outside of TextEdit and a couple places internally. Got some notes down for #31739, where instead we create new method called get_visual_line_count that handles not only wrapping but hidden lines as well.

@Paulb23
Copy link
Member

Paulb23 commented Aug 30, 2021

Closing as superseded by #50371 which exposed get_total_visible_line_count to do this.

@Paulb23 Paulb23 closed this Aug 30, 2021
@Paulb23 Paulb23 added archived and removed cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Aug 30, 2021
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.

TextEdit true Line Count and position with Autowrap.
4 participants