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

Vertical shrinking of screen beyond a limit causes app to crash #23

Closed
Mithil467 opened this issue Sep 25, 2020 · 12 comments · Fixed by #67
Closed

Vertical shrinking of screen beyond a limit causes app to crash #23

Mithil467 opened this issue Sep 25, 2020 · 12 comments · Fixed by #67
Assignees
Labels
bug Something isn't working First-timers good first issue Good for newcomers Hacktoberfest
Milestone

Comments

@Mithil467
Copy link
Owner

Steps to reproduce

Start mitype. Reduce the size of the terminal vertically. It will crash after a certain point where screen becomes smaller than what is required to print the text.

Expected behaviour

Mitype should:

  1. Print to stdout a user friendly text message like "Window too small to print given text"
  2. Exit

Actual behaviour

Crashes with traceback -

  File "/home/mithil/p/mitype/mitype/app.py", line 354, in resize
    self.setup_print(win)
  File "/home/mithil/p/mitype/mitype/app.py", line 173, in setup_print
    win.addstr(2, 0, self.text, curses.A_BOLD)
_curses.error: addwstr() returned ERR

Your proposed fix

We have implemented a size check in the initialize method in app.py already which warns the user that the terminal is smaller than the amount of lines required. We can use a similar check in resize method.

@Mithil467 Mithil467 added bug Something isn't working good first issue Good for newcomers Hacktoberfest First-timers labels Sep 25, 2020
@p4mela-g
Copy link
Contributor

Hey! I have interest in doing this one too if it's okay :)
and I'm going to open a PR during Hacktoberfest!

@Mithil467
Copy link
Owner Author

Hi @gudeliauskaspam 👋🏼

Thank you for your interest in contributing! 🎉
Yes it if fine, you may go ahead with it during hacktoberfest as well.

@p4mela-g
Copy link
Contributor

p4mela-g commented Oct 5, 2020

hey @Mithil467 , sorry for the lack of news, I will start working on this problem next week :)

@Mithil467 Mithil467 added this to the v0.2.1 milestone Oct 17, 2020
@p4mela-g
Copy link
Contributor

p4mela-g commented Oct 19, 2020

as far as i could understand the problem and the code in initialize method, a solution would be to count how many characters or maybe words fit in the box vertically.

@Mithil467
Copy link
Owner Author

Have a look at this line, this is what is being done once at the start of the program.

if self.line_count + 3 > self.window_height:

So at the start, we already are checking if window size if enough to print the text.

Similarly, we will need a check inside resize function.

def resize(self, win):

@p4mela-g
Copy link
Contributor

sorry I got it wrong, thanks for clarifying!

@p4mela-g
Copy link
Contributor

I fixed this problem and new ones appeared.
My solution, even if it is consistent in my point of view, behaves in such a way that the application closes without error whenever the resize is done.
I'm working on it, sorry for the long time.

@Mithil467
Copy link
Owner Author

No issues. Just make sure you are on the latest code on master before working!

@Mithil467 Mithil467 modified the milestones: v0.2.1, v0.2.2 Oct 29, 2020
@p4mela-g
Copy link
Contributor

hey, @Mithil467. I need your help.
I made this check

        self.text_height = word_wrap(self.original_text_formatted, self.window_height)
        
        if self.window_height < len(self.text_height) + 3:
        	curses.endwin()
        	sys.stdout.write("Window too small to print given text")
        	sys.exit(1)

When I increase or decrease the size of the terminal, it says "Window too small to print given text".
this should not happen when increasing.
it doesn't make sense for me not to work 😭

@Mithil467
Copy link
Owner Author

The issue here is, you are comparing it with len(text) but that returns the length of the string and not the vertical height of the window required.

Currently there exists a check here -

# Find number of lines required to print text

We have to do something similar in the resize function here -

self.line_count = (

@p4mela-g
Copy link
Contributor

p4mela-g commented Nov 3, 2020

Finally done :)

@Mithil467
Copy link
Owner Author

Thank you so much for the fix! @gudeliauskaspam 😄

Mithil467 pushed a commit that referenced this issue Nov 7, 2020
@Mithil467 Mithil467 modified the milestones: v0.2.2, v0.2.1 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working First-timers good first issue Good for newcomers Hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants