Skip to content

Commit

Permalink
Make the terminal line height default to normal
Browse files Browse the repository at this point in the history
Fixes #7911
  • Loading branch information
Tyriar committed Jun 21, 2016
1 parent b8ff9d7 commit a92cc54
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ configurationRegistry.registerConfiguration({
'default': 0
},
'terminal.integrated.lineHeight': {
'description': nls.localize('terminal.integrated.lineHeight', "Controls the line height of the terminal, this defaults to editor.lineHeight's value."),
'description': nls.localize('terminal.integrated.lineHeight', "Controls the line height of the terminal, this defaults to normal."),
'type': 'number',
'default': 0
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {Platform} from 'vs/base/common/platform';
import {IConfiguration} from 'vs/editor/common/config/defaultConfig';
import {IConfigurationService} from 'vs/platform/configuration/common/configuration';
import {ITerminalConfiguration} from 'vs/workbench/parts/terminal/electron-browser/terminal';
import {GOLDEN_LINE_HEIGHT_RATIO} from 'vs/editor/common/config/defaultConfig';
import {Builder} from 'vs/base/browser/builder';

const DEFAULT_ANSI_COLORS = {
Expand Down Expand Up @@ -70,8 +69,8 @@ const DEFAULT_ANSI_COLORS = {

export interface ITerminalFont {
fontFamily: string;
fontSize: number;
lineHeight: number;
fontSize: string;
lineHeight: string;
charWidth: number;
charHeight: number;
}
Expand Down Expand Up @@ -99,16 +98,16 @@ export class TerminalConfigHelper {
return DEFAULT_ANSI_COLORS[baseThemeId];
}

private measureFont(fontFamily: string, fontSize: number, lineHeight: number): ITerminalFont {
private measureFont(fontFamily: string, fontSize: string, lineHeight: string): ITerminalFont {
// Create charMeasureElement if it hasn't been created or if it was orphaned by its parent
if (!this.charMeasureElement || !this.charMeasureElement.parentElement) {
this.charMeasureElement = new Builder(this.parentDomElement, true).div().build().getHTMLElement();
}
let style = this.charMeasureElement.style;
style.display = 'inline';
style.fontFamily = fontFamily;
style.fontSize = fontSize + 'px';
style.lineHeight = lineHeight + 'px';
style.fontSize = fontSize;
style.lineHeight = lineHeight;
this.charMeasureElement.innerText = 'X';
let rect = this.charMeasureElement.getBoundingClientRect();
style.display = 'none';
Expand All @@ -133,13 +132,9 @@ export class TerminalConfigHelper {

let fontFamily = terminalConfig.fontFamily || editorConfig.editor.fontFamily;
let fontSize = this.toInteger(terminalConfig.fontSize, 0) || editorConfig.editor.fontSize;
let lineHeight = this.toInteger(terminalConfig.lineHeight, 0) || editorConfig.editor.lineHeight;
let lineHeight = this.toInteger(terminalConfig.lineHeight, 0);

if (lineHeight === 0) {
lineHeight = Math.round(GOLDEN_LINE_HEIGHT_RATIO * fontSize);
}

return this.measureFont(fontFamily, fontSize, lineHeight);
return this.measureFont(fontFamily, fontSize + 'px', lineHeight === 0 ? 'normal' : lineHeight + 'px');
}

public getShell(): IShell {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ export class TerminalInstance {
public setFont(font: ITerminalFont): void {
this.font = font;
this.terminalDomElement.style.fontFamily = this.font.fontFamily;
this.terminalDomElement.style.lineHeight = this.font.lineHeight + 'px';
this.terminalDomElement.style.fontSize = this.font.fontSize + 'px';
this.terminalDomElement.style.lineHeight = this.font.lineHeight;
this.terminalDomElement.style.fontSize = this.font.fontSize;
}

public focus(force?: boolean): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ suite('Workbench - TerminalConfigHelper', () => {
}
});
configHelper = new TerminalConfigHelper(Platform.Linux, configurationService, fixture);
assert.equal(configHelper.getFont().fontSize, 2, 'terminal.integrated.fontSize should be selected over editor.fontSize');
assert.equal(configHelper.getFont().fontSize, '2px', 'terminal.integrated.fontSize should be selected over editor.fontSize');

configurationService = new MockConfigurationService({
editor: {
Expand All @@ -97,7 +97,7 @@ suite('Workbench - TerminalConfigHelper', () => {
}
});
configHelper = new TerminalConfigHelper(Platform.Linux, configurationService, fixture);
assert.equal(configHelper.getFont().fontSize, 1, 'editor.fontSize should be the fallback when terminal.integrated.fontSize not set');
assert.equal(configHelper.getFont().fontSize, '1px', 'editor.fontSize should be the fallback when terminal.integrated.fontSize not set');

configurationService = new MockConfigurationService({
editor: {
Expand All @@ -112,7 +112,7 @@ suite('Workbench - TerminalConfigHelper', () => {
}
});
configHelper = new TerminalConfigHelper(Platform.Linux, configurationService, fixture);
assert.equal(configHelper.getFont().lineHeight, 2, 'terminal.integrated.lineHeight should be selected over editor.lineHeight');
assert.equal(configHelper.getFont().lineHeight, '2px', 'terminal.integrated.lineHeight should be selected over editor.lineHeight');

configurationService = new MockConfigurationService({
editor: {
Expand All @@ -127,7 +127,7 @@ suite('Workbench - TerminalConfigHelper', () => {
}
});
configHelper = new TerminalConfigHelper(Platform.Linux, configurationService, fixture);
assert.equal(configHelper.getFont().lineHeight, 1, 'editor.lineHeight should be the fallback when terminal.integrated.lineHeight not set');
assert.equal(configHelper.getFont().lineHeight, 'normal', 'editor.lineHeight should be "normal" when terminal.integrated.lineHeight not set');
});

test('TerminalConfigHelper - getShell', function () {
Expand Down

1 comment on commit a92cc54

@kisstkondoros
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.