fix: handle skipped questions in when with display option #1853

Sakthieswaran-tech · 2025-10-18T09:24:08Z

Display questions when when returns { ask: false, display: true } without adding to answers.
Fully skip questions when when returns { ask: false, display: false } or false.
Added tests to cover these cases.

codecov · 2025-10-18T09:25:01Z

Codecov Report

❌ Patch coverage is 93.33333% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@a171f0e). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
packages/inquirer/src/ui/skipped-renderer.ts	90.69%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1853   +/-   ##
=======================================
  Coverage        ?   94.77%           
=======================================
  Files           ?       38           
  Lines           ?     1587           
  Branches        ?      466           
=======================================
  Hits            ?     1504           
  Misses          ?       80           
  Partials        ?        3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sakthieswaran-tech · 2025-10-21T17:53:33Z

Hey @SBoudrias , I’ve added more test cases and all checks are passing. Could you take a look when you get a chance and let me know if anything needs changes?

SBoudrias

The PR is a little big, so I didn't review all of it in depth yet. Here's a few early thoughts to streamline the code a little.

packages/inquirer/src/ui/skipped-renderer.ts

packages/inquirer/package.json

README.md

packages/inquirer/src/ui/prompt.ts

Sakthieswaran-tech · 2025-10-28T08:12:28Z

Rebased with latest main.
Incorporated all the review changes (including normalization and theme updates).
The reviewed fixes are now part of the existing commits after rebase.

Sakthieswaran-tech · 2025-11-12T17:00:46Z

Hi @SBoudrias , I’ve made the requested changes and resolved the conflicts. Please take a look when you have some time and let me know if I need to make any further changes. Thanks!

Sakthieswaran-tech · 2025-12-03T17:23:24Z

Hi! Just checking in on this PR whenever you get a chance. Let me know if anything else is needed from my side.

SBoudrias

I did a slight refactoring pass on some core changes. Besides that, I think there's still a fair amount of work to properly consider how this works with plugins and how we respect user theme.

SBoudrias · 2026-01-10T20:09:52Z

packages/inquirer/src/ui/skipped-renderer.ts

+const theme = makeTheme();
+const prefix = typeof theme.prefix === 'string' ? theme.prefix : theme.prefix.idle;


Doing here mean we're only support the default theme, whereas we should match the theme if one is provided by the user.

SBoudrias · 2026-01-10T20:10:31Z

packages/inquirer/src/ui/skipped-renderer.ts

+};
+
+function renderLine(message: string, answerText: string) {
+  return theme.style.help(`${prefix} ${message} ${answerText}`);


help likely isn't the right theme key to use here; it's not semantically correct. We should add a new key to cover skipped question lines.

SBoudrias · 2026-01-10T20:11:40Z

packages/inquirer/inquirer.test.ts

    });
+
+    it('should display skipped question when `when` returns { ask: false, display: true }', async () => {
+      const writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);


I think it'll be cleaner here to reuse the BufferedStream from packages/testing/src/index.ts to collect the writes.

SBoudrias · 2026-01-10T20:14:12Z

packages/inquirer/src/ui/skipped-renderer.ts

+  confirm: (question: TypedQuestion) => {
+    const defaultVal = question.default;
+    const answerText = defaultVal === true ? 'Yes' : defaultVal === false ? 'No' : '';
+    return renderLine(question.message.toString(), answerText);
+  },


I don't have a good suggestion right now, but implementing rendering here while the actual rendering is in other package isn't great...

Ideally rendering should be consistent and provided by each prompt.

SBoudrias · 2026-01-10T20:15:15Z

packages/inquirer/src/ui/skipped-renderer.ts

+  },
+
+  default: (question: TypedQuestion) => {
+    const answerText = question.default !== undefined ? String(question.default) : '';


Given default handle ALL prompts - not just the core ones within Inquirer, I don't think we can safely assume default can be stringified; for all we know default can be an object or an array.

SBoudrias · 2026-01-10T20:16:03Z

packages/inquirer/src/ui/skipped-renderer.ts

+  },
+
+  editor: (question: TypedQuestion) => {
+    const answerText = question.default !== undefined ? '[Default Content]' : '';


What user value does [Default Content] provide here? (same with PASSWORD SET later.)

Sakthieswaran-tech force-pushed the feature/display-skipped-questions branch 2 times, most recently from fe5866b to 7f04251 Compare October 18, 2025 11:41

SBoudrias reviewed Oct 25, 2025

View reviewed changes

Sakthieswaran-tech force-pushed the feature/display-skipped-questions branch 2 times, most recently from 8e76e3c to 5b30e81 Compare October 28, 2025 08:09

Sakthieswaran-tech and others added 3 commits January 10, 2026 14:19

fix: handle skipped questions in when with display option

93a7266

test cases added for all prompts

eada2ff

refactoring

2b9d1b8

SBoudrias force-pushed the feature/display-skipped-questions branch from 219cc1e to 2b9d1b8 Compare January 10, 2026 20:07

SBoudrias reviewed Jan 10, 2026

View reviewed changes

		const theme = makeTheme();
		const prefix = typeof theme.prefix === 'string' ? theme.prefix : theme.prefix.idle;

Uh oh!

fix: handle skipped questions in when with display option #1853

Are you sure you want to change the base?

fix: handle skipped questions in when with display option #1853

Conversation

Sakthieswaran-tech commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Sakthieswaran-tech commented Oct 21, 2025

Uh oh!

SBoudrias left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Sakthieswaran-tech commented Oct 28, 2025

Uh oh!

Sakthieswaran-tech commented Nov 12, 2025

Uh oh!

Sakthieswaran-tech commented Dec 3, 2025

Uh oh!

SBoudrias left a comment

Choose a reason for hiding this comment

Uh oh!

SBoudrias Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

SBoudrias Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

SBoudrias Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

SBoudrias Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

SBoudrias Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

SBoudrias Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Sakthieswaran-tech commented Oct 18, 2025 •

edited

Loading

codecov bot commented Oct 18, 2025 •

edited

Loading