search

p-e-w / finalterm

At last – a modern terminal emulator (NO LONGER MAINTAINED)
http://finalterm.org
GNU General Public License v3.0
3.84k stars 179 forks source link

Close #Iss218 #376

Closed arkocal closed 9 years ago

arkocal commented 9 years ago

I made a few variables that should actually remain private public. If the solution is OK, I am going to resolve that using getters / setters etc. but I wanted you to have a look first because I am not quite sure if it is a good idea to "connect" those classes.

arkocal commented 9 years ago

Now there is only one public variable left, which I think is ok :smile:

p-e-w commented 9 years ago

Actually, I believe we can simplify this and some other code here. I introduced is_collapsible_start/is_collapsible_end to have a flexible solution for deciding whether there should be an arrow or not but we now know that both of these variables are really the same as output_line.is_prompt_line.

Therefore my suggestion is to get rid of those two variables entirely and replace them with checks for is_prompt_line like you are already doing here. Additionally, is_collapsible should include a check for the current line as well (i.e. whether it is a prompt line); that way, double checks like here can be avoided.

I am personally OK with linking LineView and LineContainer in the way you proposed because there really seems to be no other method to achieve the desired effect. However, following the pattern used elsewhere in the code (such as TerminalView and TerminalOutputView), the LineContainer instance should be passed to the LineView through the constructor rather than set afterwards. That way, there is no need to have any public variables at all :+1:

p-e-w commented 9 years ago

Merged! What a lovely improvement :) The UI looks so much better now!