[qt] Windows: Make rpcconsole monospace font larger #7364

Merged
merged 1 commit into from Jan 22, 2016

Conversation

Projects
None yet
4 participants
@MarcoFalke
Member

MarcoFalke commented Jan 17, 2016

Probably fixes #7017.

Merges fine into master and 0.12.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jan 17, 2016

Member

utACK for 0.12 (rc2) not sure if we want this for master/0.13 or if we should look for a better/alternative solution.

Build to test should be ready soon: https://bitcoin.jonasschnelli.ch/pulls/7364/

Member

jonasschnelli commented Jan 17, 2016

utACK for 0.12 (rc2) not sure if we want this for master/0.13 or if we should look for a better/alternative solution.

Build to test should be ready soon: https://bitcoin.jonasschnelli.ch/pulls/7364/

@jonasschnelli jonasschnelli added the GUI label Jan 17, 2016

@jonasschnelli jonasschnelli added this to the 0.12.0 milestone Jan 17, 2016

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jan 17, 2016

Member

Thanks for the binaries.

Even though this is ugly, I'd say it should be merged into master as well. Lacking a better solution, it doesn't make things worse and backports easier.

Member

MarcoFalke commented Jan 17, 2016

Thanks for the binaries.

Even though this is ugly, I'd say it should be merged into master as well. Lacking a better solution, it doesn't make things worse and backports easier.

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jan 17, 2016

Member

Why are we not using QFontInfo(fixedFontInfo).pointSize() here?

Member

luke-jr commented Jan 17, 2016

Why are we not using QFontInfo(fixedFontInfo).pointSize() here?

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jan 17, 2016

Member

@luke-jr dejavu #6864. (tl;dr too large)

Member

MarcoFalke commented Jan 17, 2016

@luke-jr dejavu #6864. (tl;dr too large)

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jan 17, 2016

Member

Seems like QFont() has the same problems? Makes more sense to scale fixedFontInfo than to scale some other font's default size. Besides, we got that Qt5 thing since then?

Member

luke-jr commented Jan 17, 2016

Seems like QFont() has the same problems? Makes more sense to scale fixedFontInfo than to scale some other font's default size. Besides, we got that Qt5 thing since then?

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jan 17, 2016

Member

Makes more sense to scale fixedFontInfo than to scale some other font's default size

Previously we had a "default font", so that's the reference. Though, you are welcome to change that.

Besides, we got that Qt5 thing since then?

Doesn't change anything for Windows or Linux. Only a different font for OS X.

Member

MarcoFalke commented Jan 17, 2016

Makes more sense to scale fixedFontInfo than to scale some other font's default size

Previously we had a "default font", so that's the reference. Though, you are welcome to change that.

Besides, we got that Qt5 thing since then?

Doesn't change anything for Windows or Linux. Only a different font for OS X.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jan 18, 2016

Member

Oh no, not more monospace related code changes. When are you guys going to give this up :-)

Member

laanwj commented Jan 18, 2016

Oh no, not more monospace related code changes. When are you guys going to give this up :-)

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jan 22, 2016

Member

Tested ACK fa6a59d.
This is a quick and stable workaround and we should backport this to 0.12.
For 0.13 we need a better solution.

Current Master (to small on windows):
bildschirmfoto 2016-01-22 um 09 39 07

Current 0.11 version, non pretty, non mono (people are use to this font size):
bildschirmfoto 2016-01-22 um 09 41 02

This PR:
bildschirmfoto 2016-01-22 um 09 38 12

Member

jonasschnelli commented Jan 22, 2016

Tested ACK fa6a59d.
This is a quick and stable workaround and we should backport this to 0.12.
For 0.13 we need a better solution.

Current Master (to small on windows):
bildschirmfoto 2016-01-22 um 09 39 07

Current 0.11 version, non pretty, non mono (people are use to this font size):
bildschirmfoto 2016-01-22 um 09 41 02

This PR:
bildschirmfoto 2016-01-22 um 09 38 12

@jonasschnelli jonasschnelli merged commit fa6a59d into bitcoin:master Jan 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

jonasschnelli added a commit that referenced this pull request Jan 22, 2016

Merge #7364: [qt] Windows: Make rpcconsole monospace font larger
fa6a59d [qt] Windows: Make rpcconsole monospace font larger (MarcoFalke)

laanwj added a commit that referenced this pull request Jan 22, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jan 22, 2016

Member

I don't think this solution is acceptable in the longer term. We have to do one of these:

  • Remove the #ifdef, just accept that the font is too big on some OSes (better too big than too small)
  • Revert the console to proportional font as in 0.10.x
Member

laanwj commented Jan 22, 2016

I don't think this solution is acceptable in the longer term. We have to do one of these:

  • Remove the #ifdef, just accept that the font is too big on some OSes (better too big than too small)
  • Revert the console to proportional font as in 0.10.x

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1601-qtWinFontSize branch Jan 22, 2016

@laanwj laanwj removed the Needs backport label Feb 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment