textual icon indicating copy to clipboard operation
textual copied to clipboard

Fix missing Tree.NodeHighlighted message

Open robinvd opened this issue 2 years ago • 8 comments
trafficstars

When the tree has no selection the cursor_line is -1, then clicking on the last node in the tree will check if the selection is different from the last selection, and do a check nodes[previous_cursor_line] != nodes[new_cursor_line] but if previous_cursor_line=-1 and new_cursor_line=len(nodes)-1 then this will fail and the message will not be emitted.

Please review the following checklist.

  • [ ] Docstrings on all new or modified functions / classes
  • [ ] Updated documentation
  • [x] Updated CHANGELOG.md (where appropriate)

robinvd avatar Oct 13 '23 10:10 robinvd

Any chance of a regression test? I'm not a maintainer of Textual, but I'm sure you'll be asked this when someone gets to reviewing this. Thanks for you PR!

TomJGooding avatar Oct 16 '23 18:10 TomJGooding

any change to review this @davep (or other maintainers)

robinvd avatar Oct 31 '23 08:10 robinvd

@robinvd We've got a lot going on so sometimes it can take us a wee while to get through PRs. Meanwhile though what Tom said is good advice: if you've identified a bug and this is a PR to fix it then a regression test would be very welcome.

davep avatar Oct 31 '23 08:10 davep

@davep ofc I understand!On 31 Oct 2023, at 09:22, Dave Pearson @.***> wrote: @robinvd We've got a lot going on so sometimes it can take us a wee while to get through PRs. Meanwhile though what Tom said is good advice: if you've identified a bug and this is a PR to fix it then a regression test would be very welcome.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

robinvd avatar Oct 31 '23 11:10 robinvd

Just to add too: having enabled the running of tests in CI: it looks like your change might be causing some existing tests to fail; you will want to take that into account.

davep avatar Oct 31 '23 14:10 davep

@robinvd Could you open an issue for this please. With a small example that reproduces the problem. Just to help me understand what this is fixing.

willmcgugan avatar Nov 29 '23 14:11 willmcgugan

Closing. Assumed stale.

willmcgugan avatar Mar 19 '24 15:03 willmcgugan

@willmcgugan repro example

from textual.app import App
from textual.widgets import Tree


class BugApp(App):
    def compose(self):
        t = Tree("test")
        t.show_root = False
        t.root.add_leaf("node1")
        t.root.add_leaf("node2")
        t.root.add_leaf("node3")
        yield t

    def on_tree_node_highlighted(self, event):
        self.notify(f"hightlight! {event}")


BugApp().run()

if you now click on the last node, no notification will popup

robinvd avatar Apr 02 '24 17:04 robinvd