dbcli / litecli

CLI for SQLite Databases with auto-completion and syntax highlighting
https://litecli.com
BSD 3-Clause "New" or "Revised" License
2.12k stars 68 forks source link

The bottom part of the screen is constantly empty #38

Closed 64kramsystem closed 5 years ago

64kramsystem commented 5 years ago

On the terminal of my machine (MATE Terminal 1.20.0), the 7 bottom lines (except the last one, which is the status bar) are constantly blank.

This looks odd, and it's also a waste of space.

I can reproduce it all the time. I use a pretty much standard configuration, with the pager disabled.

If I enable the pager, the pager output fills the entire screen, however, when I exit it, the blank space it's there again.

I have the suspicion that this is caused by the autocompletion menu "reserving" that space. At a very brief check of the source code, it seems that suggestions can't be disabled, so I can't test straight away if this is the cause.

64kramsystem commented 5 years ago

OK, it seems this is intended:

    def get_reserved_space(self):
        """Get the number of lines to reserve for the completion menu."""
        reserved_space_ratio = 0.45
        max_reserved_space = 8
        _, height = click.get_terminal_size()
        return min(int(round(height * reserved_space_ratio)), max_reserved_space)

I'm still perplexed, because I see that upward pop-up menus are supported - they print upwards when inputting multi-line sql.

All in all, I think the suggestions pop-up menu has inconsistent behavior - something else I've found is that even if the supposed reserved space is 8 (yield 7 entries), when inputting a sql statement more than 8 lines long, the menu will show (upwards) up to 15 entries.

amjith commented 5 years ago

The upward pop-up menu is a desperate attempt to show the menu when the app is all out of options. It hinders the view of the statement you've entered so far and it is generally an unpleasant experience.

That's the reason why we reserve 7 lines at the bottom. Your point about showing 15 items is using a scroll mechanism in the pop-up menu itself. It's actually not limited to 15 items, it'll show as many items as we have for that context.

I understand that it feels like a waste of space at the bottom but given the various options we chose to go with the one with minimal impact.

64kramsystem commented 5 years ago

The upward pop-up menu is a desperate attempt to show the menu when the app is all out of options. It hinders the view of the statement you've entered so far and it is generally an unpleasant experience.

That's the reason why we reserve 7 lines at the bottom. Your point about showing 15 items is using a scroll mechanism in the pop-up menu itself. It's actually not limited to 15 items, it'll show as many items as we have for that context.

I understand that it feels like a waste of space at the bottom but given the various options we chose to go with the one with minimal impact.

I see. In this case, two notes:

  1. I suspect we're not referring to the same thing regarding the 15. This is what I refer to:
/path/to/myfile.sqlite3> a 
                         a 
                         a 
                          ABORT              
                          ACTION             
                          ADD                
                          AFTER              
                          ALL                
                          ALTER              
                          ANALYZE            
                          AND                
                          AS                 
                          ASC                
                          ATTACH             
                          AUTOINCREMENT      
                          BEFORE             
                          BEGIN              
                          BETWEEN            
                          BIGINT             

this is obtained by enabling multiline statements, and typing (a -> Enter) 20 times. It's not very clear from the output, but that's a 16-lines (so I was wrong mentioning 15) menu, from a much longer list of suggestions (after the last enter, LiteCLI opens the popup for a null prefix, yielding the list of all the potential SQL commands).

Is 16 (upwards) entries intended to be different from the 7 downwards?

  1. Would you consider a PR allowing the user to customize the number of entries? I think it would be helpful in general, as people may prefer different lengths.
amjith commented 5 years ago
  1. The 7 lines is the absolute minimum we reserve. If there is more space in the screen then I believe we will expand the menu to 16 lines. I'm not sure on the exact details.
  2. Regarding the config option. Take a quick look at this design philosophy: https://fishshell.com/docs/current/design.html

I am generally reluctant about adding a kitchen sink of options in the config file. But this is a new project and I would love to have more people participating in the code development. If you feel strongly about it, feel free to open a PR, I'll see what I can do. :smile:

64kramsystem commented 5 years ago

I am generally reluctant about adding a kitchen sink of options in the config file. But this is a new project and I would love to have more people participating in the code development.

Mostly, the amount of rows left blank is relatively high. On my machine, it's 7 rows on 57 - 12%.

The option to specify the number of rows may be too fine grained. What do you think of an option to entirely remove the suggestions menu? Autocompletion is still available even without those (it happens via "shadow" characters).

amjith commented 5 years ago

Aha. Now I know the reasoning behind your request. The real crux of the issue is that 7 lines might be enough for some people but it might be too much for someone else who has a small monitor or a lower resolution. So instead of hardcoding it to 7 how about we do something a bit more adaptive? I'm sure we can easily find the size of the terminal, how about we reserve 10% of that height for the menu and have a hard limit of 7 if the 10% ends up being greater than 7?

I want to highlight this passage from the fish design document:

Every configuration option in a program is a place where the program is too stupid to figure out for itself what the user really wants, and should be considered a failure of both the program and the programmer who implemented it.

Let's see if we can do something smart instead of asking the user to choose. :smile:

If you'd like to submit that as a PR, I'll be more than happy to merge it in.

64kramsystem commented 5 years ago

I've just patched the code not to show anything, for my specific case is much more convenient.

Since the generic issue is moot (the space is intended), I think the issue can be closed, but thanks for the explanations!

amjith commented 5 years ago

:+1: