Skip to content

Conversation

sharktide
Copy link
Contributor

@sharktide sharktide commented Mar 4, 2025

Add options -nolinestop, -all, -overlap, and -strictlimits to the search function of the text class of tkinter.

Fixes #130693
Closes #130693

Tested against

.\PCbuild\amd64\python_d.exe -m test -ugui test_tkinter

And test passed. Result:

0:00:00 [1/1] test_tkinter
0:00:27 load avg: 3.93 [1/1] test_tkinter passed

== Tests result: SUCCESS ==

1 test OK.

Total duration: 27.6 sec
Total tests: run=787 skipped=11
Total test files: run=1/1
Result: SUCCESS

Also tested against a custom test file:

import tkinter as tk

root = tk.Tk()
text = tk.Text(root)
text.pack()

text.insert('1.0', 'This is a test. This is only a test.\nAnother line.\nYet another line.')

# Test without options
result = text.search('test', '1.0', 'end')
print('Without options:', result)

# Test with -nolinestop
result = text.search('line', '1.0', 'end', nolinestop=True, regexp=True)
print('With -nolinestop:', result)

# Test with -all
result = text.search('test', '1.0', 'end', all=True)
print('With -all:', result)

# Test with -overlap
result = text.search('test', '1.0', 'end', overlap=True, all=True)
print('With -overlap:', result)

# Test with -strictlimits
result = text.search('test', '1.0', 'end', strictlimits=True)
print('With -strictlimits:', result)

root.mainloop()

Which returns:

Without options: 1.10
With -nolinestop: 2.8
With -all: (<textindex object: '1.10'>, <textindex object: '1.31'>)
With -overlap: (<textindex object: '1.10'>, <textindex object: '1.31'>)
With -strictlimits: 1.10

Indicating that the new features are functioning correctly

@bedevere-app
Copy link

bedevere-app bot commented Mar 4, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@sharktide sharktide requested a review from Wulian233 March 5, 2025 15:20
@sharktide
Copy link
Contributor Author

@ZeroIntensity Could someone please review? I think the original reviewer forgot

@sharktide sharktide requested a review from Wulian233 May 5, 2025 15:49
@sharktide
Copy link
Contributor Author

@Wulian233 Please Review again

@sharktide
Copy link
Contributor Author

I am currently pinging this PR as I have not seen a review in nearly a month.
Could someone please review?

@sharktide
Copy link
Contributor Author

sharktide commented May 30, 2025 via email

@sharktide
Copy link
Contributor Author

Done!

@sharktide sharktide requested a review from ZeroIntensity May 30, 2025 20:09
@sharktide
Copy link
Contributor Author

It has been nearly a month. @ZeroIntensity Could you please review?

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

All new features need tests. Please add one to test_tkinter.

@sharktide
Copy link
Contributor Author

sharktide commented Jun 17, 2025 via email

@sharktide
Copy link
Contributor Author

I updated the branch as it has been a while since this PR has been opened

@sharktide sharktide requested a review from AA-Turner as a code owner August 10, 2025 21:54
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Thank you for your update.

@sharktide
Copy link
Contributor Author

Gentle ping

^^^^^^^^^^^^^^
AttributeError: 'Container' object has no attribute 'area'. Did you mean: 'inner.area'?
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change.

'Another line.\n'
'Yet another line.')

result = text.search('line', '1.0', 'end', nolinestop=True, regexp=True)
Copy link
Member

Choose a reason for hiding this comment

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

Why regexp=True if it is not a regexp?

It would be nice for each option to call search() with the same arguments, but with and without that option (the results should be different). So we will see that it works.

Comment on lines +73 to +83
self.assertGreater(len(overlap_res), len(all_res))

# Check that overlap actually finds overlapping matches
self.assertIn('2.0', overlap_res_strs)
self.assertIn('2.2', overlap_res_strs)
self.assertIn('2.4', overlap_res_strs)
self.assertNotIn('2.2', all_res_strs)

# Ensure all results are valid text indices
for i in overlap_res:
self.assertRegex(str(i), r'^\d+\.\d+$')
Copy link
Member

Choose a reason for hiding this comment

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

Can't we simply test self.assertEqual(overlap_res_strs, ['1.0', '1.2', ...])? And the same for non-overlapped search. The example should perhaps be smaller.


(Contributed by Pablo Galindo and László Kiss Kollár in :gh:`135953`.)

=======
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change.

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.

tkinter missing -nolinestop on tk.Text

5 participants