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

Tree.scroll_to_line() off-by-2 error in x coordinate? #4741

Closed
rcrowell opened this issue Jul 15, 2024 · 3 comments · Fixed by #4744
Closed

Tree.scroll_to_line() off-by-2 error in x coordinate? #4741

rcrowell opened this issue Jul 15, 2024 · 3 comments · Fixed by #4744
Labels
bug Something isn't working

Comments

@rcrowell
Copy link

rcrowell commented Jul 15, 2024

Have you checked closed issues? https://github.com/Textualize/textual/issues?q=is%3Aissue+is%3Aclosed

Yes.

Description

Pressing up/down to change the highlighted row in a Tree widget will scroll to strange x-offsets when node labels are wide.

For instance, render this Tree:
Selection_001

And then press the down key to highlight the first leaf of root:
Selection_002

Expected: Content scrolled so that "a" is the first visible character in the first leaf
Observed: Content scrolled so that "c" is the first visible character in the first leaf

Here is my example app:

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


class ExampleApp(App):
    CSS = '''
    Tree {
        width: 16;
    }
    '''

    def compose(self):
        tree = Tree(label='root')
        tree.root.add_leaf('abcdefghijklmnopqrstuvwxyz')
        child = tree.root.add('antidisestablishmentarianism')
        child.add_leaf('interesting')
        child.add_leaf('very interesting')
        child.add_leaf('very very interesting')
        tree.root.add_leaf('not long')
        tree.root.add_leaf('looooooooooooooooong')
        # Expand nodes.
        tree.root.expand()
        child.expand()
        yield tree

    
if __name__ == '__main__':
    app = ExampleApp()
    app.run()

Possible Cause

This is the function in question:

class _TreeLine(Generic[TreeDataType]):
    def _get_guide_width(self, guide_depth: int, show_root: bool) -> int:                                                                                                                                                                                                                      
        if show_root:
            return 2 + (max(0, len(self.path) - 1)) * guide_depth
        else:
            guides = 2
            if len(self.path) > 1:
                guides += (len(self.path) - 1) * guide_depth

        return guides

Questions:

  1. What is the significance of "2" in the above? Was this intended to ensure the down arrow is visible for non-leaf nodes? Or perhaps the DirectoryTree widget relies on this to show the file/folder icons?
  2. Kind of an aside, but is there any case where the boolean value of show_root changes the result? Is the calculation not the same in either case?

Fix?

rcrowell@ec2207c seems to fix the off-by-two error in my testing. I may be missing something super obvious though, as this is my first weekend using textual...

Diagnostic

$ textual diagnose
<!-- This is valid Markdown, please paste the following directly in to a GitHub issue -->
# Textual Diagnostics

## Versions

| Name    | Value  |
|---------|--------|
| Textual | 0.71.0 |
| Rich    | 13.7.1 |

## Python

| Name           | Value                                     |
|----------------|-------------------------------------------|
| Version        | 3.10.4                                    |
| Implementation | CPython                                   |
| Compiler       | GCC 11.2.0                                |
| Executable     | /home/rcrowell/bemix/bemix-env/bin/python |

## Operating System

| Name    | Value                                                   |
|---------|---------------------------------------------------------|
| System  | Linux                                                   |
| Release | 6.5.11-8-pve                                            |
| Version | #1 SMP PREEMPT_DYNAMIC PMX 6.5.11-8 (2024-01-30T12:27Z) |

## Terminal

| Name                 | Value          |
|----------------------|----------------|
| Terminal Application | *Unknown*      |
| TERM                 | xterm-256color |
| COLORTERM            | *Not set*      |
| FORCE_COLOR          | *Not set*      |
| NO_COLOR             | *Not set*      |

## Rich Console options

| Name           | Value               |
|----------------|---------------------|
| size           | width=80, height=24 |
| legacy_windows | False               |
| min_width      | 1                   |
| max_width      | 80                  |
| is_terminal    | True                |
| encoding       | utf-8               |
| max_height     | 24                  |
| justify        | None                |
| overflow       | None                |
| no_wrap        | False               |
| highlight      | None                |
| markup         | None                |
| height         | None                |
Copy link

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

@darrenburns
Copy link
Member

Good catch - thanks. I've opened a PR for this.

Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants