To fix this, we'll need to mark encapsulateSelection() as public

It's not just encapsulateSelection(), it's all methods under fn (getContents, setContents, getSelection...).

How weird, I wasn't seeing those in JSDuck before. Thanks!

Correction: We'll need to mark these methods as public:

• getContents
• setContents
• getSelection
• replaceSelection
• encapsulateSelection
• getCaretPosition
• setSelection
• scrollToCaretPosition
• register
• unregister

This may affect more than just the documentation. In JSDuck, these methods, documented as @private, were available (if one clicked on a link in the textSelection() documentation or checked Private in the Show menu) while they were clearly marked as something you shouldn’t (and, in fact, can’t) use directly. In JSDoc, it was decided that private methods don’t appear at all in the documentation. I see three options:

• Copy the documentation of these methods to the textSelection() documentation. Easy to do, but easily causes problems later (the two copies get out of sync) and isn’t structured and thus doesn’t help IDEs.
• Reconsider the decision to exclude @private methods from the documentation. This requires major changes in the WMF JSDoc theme: not only should @private methods not be ignored, they should also be hidden by default (a concept the JSDuck interface has but the JSDoc one doesn’t currently), and even if unhidden, they should be clearly marked as private (e.g. using a chip). In this case, the only thing that needs to happen in the MediaWiki core repo is updating the JSDoc theme in package.json. This allows keeping the documentation structured but still doesn’t help IDEs (as the IDE sees only that the signature of textSelection is (command: string, commandOptions?: any) => any).
• Rethink this API from the ground up (of course with backward compatibility preserved): instead of adding content using $elem.textSelection('encapsulateSelection', {pre: '[[', post: ']]'}), add it using $elem.textSelectionEncapsulate({pre: '[[', post: ']]'}). With this, $.fn.textSelectionEncapsulate() is public and can be displayed in the documentation (and linked to). It can also documented in a way that can be understood by IDEs.

This means that the documentation is somewhat machine-readable (although there is no machine-readable connection between the command being scrollToCaretPosition and the options being a scrollToCaretPositionCommandOptions object) but still partly duplicated – the content after one of: needs to be copy-pasted from the actual description and thus can get out of sync.

Do the functions even need to be documented? They're anonymous functions inside a public method after all. (But also: do they need to be initialized from scratch on every call?)

In T358290#9593125, @Nardog wrote:

Do the functions even need to be documented? They're anonymous functions inside a public method after all.

If only the public method has a documentation, I think it’s even more likely that someone will forget to update that because that’s far from the actual code – which doesn’t lead to inconsistency, but consistently wrong documentation isn’t better than inconsistently wrong.

(But also: do they need to be initialized from scratch on every call?)

They don’t need to. However, it’s easier to do so, and it’s not definitely worse performance-wise either:

• If they’re initialized from scratch on every call, that makes each call a bit slower.
• If they’re initialized only once, they (along with their closures) remain in memory for the whole editing session.

Given that this API is relatively rarely used (understand: probably less then once per editing session in average, and even when heavily used, not more often than once in every few seconds), the win in memory footprint may outweigh the loss in speed. However, if moving them out of the function body allows making them public and publicly documented, it’s probably a compelling reason.

In T358290#9593071, @Tacsipacsi wrote:

This means that the documentation is somewhat machine-readable (although there is no machine-readable connection between the command being scrollToCaretPosition and the options being a scrollToCaretPositionCommandOptions object) but still partly duplicated – the content after one of: needs to be copy-pasted from the actual description and thus can get out of sync.

I know, that's not ideal, but I'm not sure how to better duplicate this without changing/breaking the API

rewrite the documentation using typedefs describing the options for each of the commands.

That would not be sufficient because any callback added via register() should behave the same way as its default counterpart, i.e. accept the same options and return the same expected value. Not just the options but the methods overall need to be documented.

Change #1037442 had a related patch set uploaded (by Nardog; author: Nardog):

[mediawiki/core@master] textSelection: Stop initializing default methods on each call

https://gerrit.wikimedia.org/r/1037442

Change #1037442 merged by jenkins-bot:

[mediawiki/core@master] textSelection: Stop initializing default methods on each call

https://gerrit.wikimedia.org/r/1037442