Fix split terminal did not preverse font scale #1747

[ChristianCelora]

Following #1747, i saved the current font scale so when a terminal is split the font scale is not resized to default value.

[mlouielu]

Thanks for your contribution, some points need to be polished:

1. Commit message should write more clearly, for example: Fix split terminal should preserve font scale #1747
2. Should this be update by font_scale? we may no need to add another attribute for this.

[ChristianCelora]

Thanks for the review. I am new to the open source world.

About the second point, i had found another solution, without creating a new attribute, by modifying the split() function and using the font_scale.

[mlouielu]

I will review in next few day. But my intuition said maybe we can just set font and scale anyway, the if guard may be unnecessary.

[ChristianCelora]

Hi,

do you had a chance reviewing the PR?

I tried to remove the if guard myself but when i try to open guake for the first time it returns an error on the self.terminal being equals to None.

Here the logs printed during the test run:

INFO     Guake not running, starting it
INFO     Loading Gnome schema from: /home/christian/Documents/python/personal/guake/guake/data
INFO     Language previously loaded from: /home/christian/Documents/python/personal/guake/guake/po
INFO     Guake Terminal 3.8.6.dev1
INFO     VTE 0.60.3
INFO     Gtk 3.24.20
INFO     created fresh notebook for workspace 0
INFO     Spawning new terminal at /home/christian
INFO     current workspace is 0
WARNING  can't bind show-focus key
INFO     Guake tabs restored from /home/christian/.config/guake/session.json
INFO     Guake initialized
INFO     Spawning new terminal at /home/christian
Traceback (most recent call last):
  File "/home/christian/Documents/python/personal/guake/guake/guake_app.py", line 583, in show_hide
    self.show()
  File "/home/christian/Documents/python/personal/guake/guake/guake_app.py", line 743, in show
    self.restore_pending_terminal_split()
  File "/home/christian/Documents/python/personal/guake/guake/guake_app.py", line 672, in restore_pending_terminal_split
    root.restore_box_layout(box, panes)
  File "/home/christian/Documents/python/personal/guake/guake/boxes.py", line 262, in restore_box_layout
    box = box.split_h()
  File "/home/christian/Documents/python/personal/guake/guake/boxes.py", line 460, in split_h
    return self.split(DualTerminalBox.ORIENT_V)
  File "/home/christian/Documents/python/personal/guake/guake/boxes.py", line 485, in split
    terminal.set_font(self.terminal.font)
AttributeError: 'NoneType' object has no attribute 'font'

[mlouielu]

Everything looks fine, but 2 points need to be address:

1. The block comment for if statement should put under it
2. Commit message should be polished, please try to use git rebase -i to edit them, and force-push to GitHub.

[ChristianCelora]

Hi,

i tried to rebase the commits. I did pick the first one and squashed the other two into the first one.

I am not an expert on git commands (also really scared of using them 🤣). Tell me if i did something wrong.

Christian.

[Davidy22]

I left this to louie since they were on it already and I was busy at the time, but looks like no reply in a little while and I'm digging through stuff on the repo again. Your commit log has three commits, one initial implementation, a reversion and a final implementation and I believe you could clean it up by just dropping the first two commits, but I also don't have a problem with squashing on pull and the change looks fine to me too so I'll do that.

EDIT: Just kidding, apparently squash and merge is just straight disabled here so I am going to have to request a little commit log cleanup. You can either squash the last three commits in your branch, or drop the first two and that'll get us to the clean log that we're looking for.

[ChristianCelora]

I polished the commits, squashing them into one single commit, but now the deepsource analysis fails at a commented line that i did not touch, asking to remove it.

Do i have to remove it?

Resolved

[Davidy22]

I got another pr to sweep up the stuff that deepsource has recently decided to start complaining about, don't worry about that. Looks good, merging.