Skip to content

Commit aa51538

Browse files
committed
Fix clearing in some cases
Fixes #121
1 parent 9125620 commit aa51538

File tree

4 files changed

+169
-27
lines changed

4 files changed

+169
-27
lines changed

index.js

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class Ora {
1515
#lineCount = 0;
1616
#frameIndex = -1;
1717
#lastSpinnerFrameTime = 0;
18+
#lastIndent = 0;
1819
#options;
1920
#spinner;
2021
#stream;
@@ -162,44 +163,44 @@ class Ora {
162163
return this.#id !== undefined;
163164
}
164165

165-
#getFullPrefixText(prefixText = this.#prefixText, postfix = ' ') {
166-
if (typeof prefixText === 'string' && prefixText !== '') {
167-
return prefixText + postfix;
168-
}
169-
170-
if (typeof prefixText === 'function') {
171-
return prefixText() + postfix;
166+
#formatAffix(value, separator, placeBefore = false) {
167+
const resolved = typeof value === 'function' ? value() : value;
168+
if (typeof resolved === 'string' && resolved !== '') {
169+
return placeBefore ? (separator + resolved) : (resolved + separator);
172170
}
173171

174172
return '';
175173
}
176174

175+
#getFullPrefixText(prefixText = this.#prefixText, postfix = ' ') {
176+
return this.#formatAffix(prefixText, postfix, false);
177+
}
178+
177179
#getFullSuffixText(suffixText = this.#suffixText, prefix = ' ') {
178-
if (typeof suffixText === 'string' && suffixText !== '') {
179-
return prefix + suffixText;
180-
}
180+
return this.#formatAffix(suffixText, prefix, true);
181+
}
181182

182-
if (typeof suffixText === 'function') {
183-
return prefix + suffixText();
183+
#computeLineCountFrom(text, columns) {
184+
let count = 0;
185+
for (const line of stripAnsi(text).split('\n')) {
186+
count += Math.max(1, Math.ceil(stringWidth(line) / columns));
184187
}
185188

186-
return '';
189+
return count;
187190
}
188191

189192
#updateLineCount() {
190193
const columns = this.#stream.columns ?? 80;
191194

192-
// Use simple approximations for line calculation to avoid calling functions
195+
// Simple side-effect free approximation (do not call functions)
193196
const prefixText = typeof this.#prefixText === 'function' ? '' : this.#prefixText;
194197
const suffixText = typeof this.#suffixText === 'function' ? '' : this.#suffixText;
195-
const fullPrefixText = (typeof prefixText === 'string' && prefixText !== '') ? prefixText + '-' : '';
196-
const fullSuffixText = (typeof suffixText === 'string' && suffixText !== '') ? '-' + suffixText : '';
197-
const fullText = ' '.repeat(this.#indent) + fullPrefixText + '--' + this.#text + '--' + fullSuffixText;
198+
const fullPrefixText = (typeof prefixText === 'string' && prefixText !== '') ? prefixText + ' ' : '';
199+
const fullSuffixText = (typeof suffixText === 'string' && suffixText !== '') ? ' ' + suffixText : '';
200+
const spinnerChar = '-';
201+
const fullText = ' '.repeat(this.#indent) + fullPrefixText + spinnerChar + (typeof this.#text === 'string' ? ' ' + this.#text : '') + fullSuffixText;
198202

199-
this.#lineCount = 0;
200-
for (const line of stripAnsi(fullText).split('\n')) {
201-
this.#lineCount += Math.max(1, Math.ceil(stringWidth(line, {countAnsiEscapeCodes: true}) / columns));
202-
}
203+
this.#lineCount = this.#computeLineCountFrom(fullText, columns);
203204
}
204205

205206
get isEnabled() {
@@ -264,11 +265,11 @@ class Ora {
264265
this.#stream.clearLine(1);
265266
}
266267

267-
if (this.#indent || this.lastIndent !== this.#indent) {
268+
if (this.#indent || this.#lastIndent !== this.#indent) {
268269
this.#stream.cursorTo(this.#indent);
269270
}
270271

271-
this.lastIndent = this.#indent;
272+
this.#lastIndent = this.#indent;
272273
this.#linesToClear = 0;
273274

274275
return this;
@@ -282,15 +283,16 @@ class Ora {
282283
this.clear();
283284

284285
let frameContent = this.frame();
285-
let actualLineCount = this.#lineCount;
286+
const columns = this.#stream.columns ?? 80;
287+
let actualLineCount = this.#computeLineCountFrom(frameContent, columns);
286288

287289
// If content would exceed viewport height, truncate it to prevent garbage
288290
const consoleHeight = this.#stream.rows;
289-
if (consoleHeight && consoleHeight > 1 && this.#lineCount > consoleHeight) {
291+
if (consoleHeight && consoleHeight > 1 && actualLineCount > consoleHeight) {
290292
const lines = frameContent.split('\n');
291293
const maxLines = consoleHeight - 1; // Reserve one line for truncation message
292294
frameContent = [...lines.slice(0, maxLines), '... (content truncated to fit terminal)'].join('\n');
293-
actualLineCount = maxLines + 1; // Truncated lines + message line
295+
actualLineCount = this.#computeLineCountFrom(frameContent, columns);
294296
}
295297

296298
this.#stream.write(frameContent);

index.test-d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ ora({spinner: 'squish'});
1313
ora({spinner: {frames: ['-', '+', '-']}});
1414
ora({spinner: {interval: 80, frames: ['-', '+', '-']}});
1515
ora({color: 'cyan'});
16+
ora({color: false});
1617
ora({hideCursor: true});
1718
ora({indent: 1});
1819
ora({interval: 80});
@@ -22,6 +23,7 @@ ora({isSilent: false});
2223
ora({discardStdin: true});
2324

2425
spinner.color = 'yellow';
26+
spinner.color = false;
2527
spinner.text = 'Loading rainbows';
2628
expectType<boolean>(spinner.isSpinning);
2729
spinner.spinner = 'dots';

readme.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ Type: `string | boolean`\
8080
Default: `'cyan'`\
8181
Values: `'black' | 'red' | 'green' | 'yellow' | 'blue' | 'magenta' | 'cyan' | 'white' | 'gray' | boolean`
8282

83-
The color of the spinner.
83+
The color of the spinner. Set to `false` to disable coloring.
8484

8585
##### hideCursor
8686

test.js

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,18 @@ test('.stopAndPersist() with prefixText and suffixText', macro, spinner => {
434434
spinner.stopAndPersist({symbol: '@', text: 'foo'});
435435
}, /bar @ foo baz\n$/, {prefixText: 'bar', suffixText: 'baz'});
436436

437+
test('.stopAndPersist() with dynamic prefixText and suffixText', macro, spinner => {
438+
spinner.stopAndPersist({symbol: '#', text: 'work'});
439+
}, /pre # work post\n$/, {prefixText: () => 'pre', suffixText: () => 'post'});
440+
441+
test('.stopAndPersist() with dynamic empty prefixText and suffixText has no stray spaces', macro, spinner => {
442+
spinner.stopAndPersist({symbol: '#', text: 'work'});
443+
}, /# work\n$/, {prefixText: () => '', suffixText: () => ''});
444+
445+
test('.stopAndPersist() with empty symbol does not add separator', macro, spinner => {
446+
spinner.stopAndPersist({symbol: '', text: 'done'});
447+
}, /done\n$/, {});
448+
437449
// New clear method tests
438450

439451
const currentClearMethod = transFormTTY => {
@@ -1173,6 +1185,79 @@ test('frame() should display both dynamic prefixText and suffixText from functio
11731185
t.not(frame1, frame2);
11741186
});
11751187

1188+
test('frame() should not add leading space when prefixText() returns empty', t => {
1189+
const spinner = ora({
1190+
text: 'test',
1191+
prefixText: () => '',
1192+
color: false,
1193+
});
1194+
1195+
const frame = spinner.frame();
1196+
// First character should be the spinner frame, not a space
1197+
t.not(frame[0], ' ');
1198+
});
1199+
1200+
test('frame() should not add trailing space when suffixText() returns empty', t => {
1201+
const spinner = ora({
1202+
text: 'test',
1203+
suffixText: () => '',
1204+
color: false,
1205+
});
1206+
1207+
const frame = spinner.frame();
1208+
t.false(frame.endsWith(' '));
1209+
});
1210+
1211+
test('render() uses actual content for line counts with dynamic prefixText', t => {
1212+
const stream = getPassThroughStream();
1213+
stream.isTTY = true;
1214+
stream.columns = 10;
1215+
1216+
let call = 0;
1217+
const spinner = ora({
1218+
stream,
1219+
text: 'hello',
1220+
prefixText() {
1221+
call++;
1222+
return call === 1 ? 'p' : 'pppppppppppp';
1223+
},
1224+
color: false,
1225+
isEnabled: true,
1226+
});
1227+
1228+
spinner.render();
1229+
const first = spinner._linesToClear;
1230+
spinner.render();
1231+
const second = spinner._linesToClear;
1232+
1233+
t.true(second >= first);
1234+
});
1235+
1236+
test('render() uses actual content for line counts with dynamic suffixText', t => {
1237+
const stream = getPassThroughStream();
1238+
stream.isTTY = true;
1239+
stream.columns = 10;
1240+
1241+
let call = 0;
1242+
const spinner = ora({
1243+
stream,
1244+
text: 'hello',
1245+
suffixText() {
1246+
call++;
1247+
return call === 1 ? '' : 'ssssssssssss';
1248+
},
1249+
color: false,
1250+
isEnabled: true,
1251+
});
1252+
1253+
spinner.render();
1254+
const first = spinner._linesToClear;
1255+
spinner.render();
1256+
const second = spinner._linesToClear;
1257+
1258+
t.true(second >= first);
1259+
});
1260+
11761261
test('frame() should handle mixed static and dynamic text', t => {
11771262
let counter = 0;
11781263
const spinner = ora({
@@ -1236,3 +1321,56 @@ test('frame() functions should only be called during frame() execution, not duri
12361321
t.is(constructorCalls, 2);
12371322
t.true(frame2.includes('Called 2'));
12381323
});
1324+
1325+
test('updateLineCount() does not call prefix/suffix functions when changing text', t => {
1326+
let calls = 0;
1327+
const spinner = ora({
1328+
text: 'test',
1329+
prefixText() {
1330+
calls++;
1331+
return 'pref';
1332+
},
1333+
suffixText() {
1334+
calls++;
1335+
return 'suf';
1336+
},
1337+
color: false,
1338+
});
1339+
1340+
// Change text which triggers #updateLineCount(); functions must not run
1341+
spinner.text = 'changed';
1342+
t.is(calls, 0);
1343+
1344+
// Only frame() should call them
1345+
spinner.frame();
1346+
t.is(calls, 2);
1347+
});
1348+
1349+
test('dynamic prefix can trigger truncation', t => {
1350+
const stream = getPassThroughStream();
1351+
stream.rows = 5;
1352+
stream.columns = 80;
1353+
stream.isTTY = true;
1354+
1355+
let wrote = '';
1356+
const originalWrite = stream.write;
1357+
stream.write = function (content) {
1358+
wrote = content;
1359+
return originalWrite.call(this, content);
1360+
};
1361+
1362+
const spinner = ora({
1363+
stream,
1364+
text: 'base',
1365+
prefixText: () => Array.from({length: 20}, (_, i) => `L${i + 1}`).join('\n'),
1366+
color: false,
1367+
isEnabled: true,
1368+
});
1369+
1370+
spinner.start();
1371+
spinner.render();
1372+
1373+
t.true(wrote.includes('(content truncated to fit terminal)'));
1374+
1375+
spinner.stop();
1376+
});

0 commit comments

Comments
 (0)