feat: add iterateComments method

[fisker]

PR Checklist

• Addresses an existing open issue: fixes 🚀 Feature: Replace forEach... methods with Iterables #755
• That issue was marked as status: accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

This contains change from #756, if we decide to add them one by one, should merge #756 first.

[JoshuaKGoldberg]

LGTM in general - with one note on the missing text.

Again, apologies for taking so long on this. Once I get confirmation on the intent with text I can merge and release. 👍

[JoshuaKGoldberg]

[Bug] text missing in the output is causing the tests to fail. I think text was meant to be included - is something like this missing?

Suggested change

	comments.push({ end, fullText, kind, pos });
	comments.push({ end, fullText, kind, pos, text: fullText.slice(pos, end) });

export type Comment = ts.CommentRange & {
	fullText: string;
+	text: string;
};

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.46%. Comparing base (f6fda9b) to head (20666e5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #758      +/-   ##
==========================================
+ Coverage   61.13%   61.46%   +0.32%     
==========================================
  Files          35       35              
  Lines        2923     2948      +25     
  Branches      468      473       +5     
==========================================
+ Hits         1787     1812      +25     
  Misses       1136     1136              

Flag	Coverage Δ
4.8.4	60.07% <100.00%> (+0.34%)	⬆️
latest	60.71% <100.00%> (+0.33%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

[fisker]

I don't remember what I was thinking. But it currently yields

{
	kind: CommentKind;
    pos: number;
    end: number;
	fullText: string;
}

Do you have a preferences that what it should look like?

fullText seems not very useful to me.

[JoshuaKGoldberg]

Hmm, yeah, I think removing fullText and adding back text is what I'd prefer.

[fisker]

Do we want add .value too? Can be useful for typescript-estree https://github.com/typescript-eslint/typescript-eslint/blob/c1980522cd11f2de1a49ff6a30b4be7765a843ff/packages/typescript-estree/src/convert-comments.ts#L44C9-L44C14

[JoshuaKGoldberg]

Is that value not just the same thing as text?

[fisker]

No,

// foo

{
	text: '// foo',
	value: ' foo'
}

/* foo */

{
	text: '/* foo */',
	value: ' foo '
}

Unless you mean to provide value instead of text.

[JoshuaKGoldberg]

OH! Gotcha - I think it'd be useful to provide both. At least text is good for the full text, and value is nice as friendly time-saver for users. But if you feel strongly about just providing text, I wouldn't be upset about just providing it for now.

[fisker]

Let's add value then.

[fisker]

Is the test failure related? Or should I ignore?

[JoshuaKGoldberg]

You can ignore them, I just filed #768.

[JoshuaKGoldberg]

💯 lovely, thank you!

By the way, I filed #767 to group together pending changes for a next major in perhaps a couple of months. That includes the #588 -> #711 feature you'd suggested a bit back. If there's anything else you're interested in, I'm on deck to help get it in. 🙂 lmk!

[JoshuaKGoldberg]

Published in ts-api-utils@2.3.0 🚀