Input icon indicating copy to clipboard operation
Input copied to clipboard

Featherize - my poor attempt to make Input Feather-compatible

Open gnysek opened this issue 1 year ago • 4 comments

Updates JSDoc to make it more compatible with Feather. None of __xxx functions were updated, but about 160 main ones were. Deprecated ones are marked with /// @ignore.

gnysek avatar Aug 02 '22 08:08 gnysek

Just FYI I believe feather supports /// @deprecated so perhaps that would be more appropriate for deprecated functions than /// @ignore. Not sure how feather displays deprecated functions currently though.

FaultyFunctions avatar Aug 03 '22 09:08 FaultyFunctions

This will take me a while to review, bear with me.

JujuAdams avatar Aug 03 '22 09:08 JujuAdams

Just FYI I believe feather supports /// @deprecated so perhaps that would be more appropriate for deprecated functions than /// @ignore. Not sure how feather displays deprecated functions currently though.

For functions marked as deprecated, a warning will be displayed if you attempt to use them, and the tooltip should contain an additional message letting you know the function/variable is deprecated.

katsaii avatar Aug 07 '22 13:08 katsaii

In our situation, use of @ignore is suitable for a few functions, @deprecated is suitable for others.

I cannot accept this PR for the following reasons:

  1. Parameters should not be prefixed with underscores
  2. Parameters should be in camelCase
  3. Optional parameters should be marked with square brackets along with their default value (e.g. [playerIndex=0]) so that autocomplete is still viable for developers not using Feather
  4. Parameter names should not be substantively renamed (e.g. successCallback -> _success_method)

Also I noticed a couple typing mistakes (input_profile_copy() uses strings for the profile names to copy to/from, not reals) but they're few and far between so not a dealbreaker.

If you're willing to make these changes I can merge this PR in, and I'm keen to get better Feather compatibility for the sake of futureproofing the library. But if that's too much work then that's ok too, I'll get round to it myself sooner or later.

JujuAdams avatar Aug 13 '22 10:08 JujuAdams

Problem with @param names is that Feather shows a warning if param name isn't same as inside function, that's why I changed camelCase and [optionalParameters] to what they are now.

gnysek avatar Aug 17 '22 22:08 gnysek

obraz

I've also tried with /// @func but while this work for bottom bar of code editor, Feather ignores it.

When using same variable names in JSDoc that in function definition, optional arguments are in brackets, however default value isn't shown either in bottom bar nor on hover - so that's something that YYG need to fix/add probably....

obraz

gnysek avatar Aug 17 '22 22:08 gnysek

Well that's a pain in the arse. I will have a think about how to best deal with this.

JujuAdams avatar Aug 18 '22 15:08 JujuAdams

maaaybe this should be reported as bug, as this was allowed before, and now that feature seems to be removed

gnysek avatar Aug 18 '22 17:08 gnysek