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

Improved TextEdit search usability & documentation #33202

Merged

Conversation

pouleyKetchoupp
Copy link
Contributor

These changes make search() functionality usable in scripts by defining how to get the line and column numbers from the returned PoolIntArray.

@@ -5443,11 +5443,11 @@ int TextEdit::_get_column_pos_of_word(const String &p_key, const String &p_searc
PoolVector<int> TextEdit::_search_bind(const String &p_key, uint32_t p_search_flags, int p_from_line, int p_from_column) const {

int col, line;
if (search(p_key, p_search_flags, p_from_line, p_from_column, col, line)) {
if (search(p_key, p_search_flags, p_from_line, p_from_column, line, col)) {
Copy link
Member

Choose a reason for hiding this comment

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

This swaps line and column, was it erroneous previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the arguments were previously inverted.

@akien-mga
Copy link
Member

It seems a bit convoluted to have to use enum values to index a 2-value array.
How about making result a Dictionary with line and column keys instead? (Breaks compat, so for 4.0.)

@pouleyKetchoupp
Copy link
Contributor Author

Ok, I was trying to keep compatibility but I don't mind changing the returned value to a dictionary.

@akien-mga
Copy link
Member

Well if this PR is actually a bugfix on the order of line/col, we can likely merge as is for 3.2, and improve the API further for 4.0 (probably as part of #31739).

@pouleyKetchoupp
Copy link
Contributor Author

Sounds good!

@akien-mga akien-mga added this to the 3.2 milestone Nov 1, 2019
@akien-mga akien-mga merged commit a49c8d4 into godotengine:master Nov 1, 2019
@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.

4 participants