Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Nodejs plugins not working #2557

Merged

Conversation

Yusyuriv
Copy link
Member

@Yusyuriv Yusyuriv commented Feb 10, 2024

Fixes #2553. Nodejs plugins currently aren't working. The reason is always the same: it throws an index out of bounds exception on line 31.
Error screenshot 1 (npm)
Error screenshot 2 (mdn)
Error screenshot 3 (beat)
Error screenshot 4 (hn)
Error screenshot 5 (exception)

I added line 45 just like in PythonPlugin.cs and this exception disappeared. Even though the exception disappeared, plugins still don't work, now for another reason: they except the JSON they receive to have lower-case keys, and they receive them capitalized (Method instead of method, Parameters instead of parameters etc). Now they just don't respond with anything.
Error screenshot 6 (npm)

After I added the same serialization options that were used for C# and Python plugins (lines 31 and 38), Nodejs plugins started working:
Success screenshot 1 (beat)
Success screenshot 2 (hn)
Success screenshot 3 (npm)

Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (15)
axig
EKt
GNnr
JDOI
Kad
MSb
OKifx
Qcki
QNvx
Rrl
Ukn
wej
WSg
XMw
Yzs
To accept these unrecognized words as correct, you could run the following commands

... in a clone of the git@github.com:Yusyuriv/Flow.Launcher.git repository
on the fix-nodejs-plugin-system branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/prerelease/apply.pl' |
perl - 'https://github.com/Flow-Launcher/Flow.Launcher/actions/runs/7855426412/attempts/1'
Warnings (1)

See the 📂 files view, the 📜action log, or 📝 job summary for details.

ℹ️ Warnings Count
ℹ️ non-alpha-in-dictionary 10

See ℹ️ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

@taooceros
Copy link
Member

taooceros commented Feb 11, 2024

Do executable plugin also have this issue?

@Yusyuriv
Copy link
Member Author

No, they do not. They create space in the ArgumentList collection in constructor (ExecutablePlugin.cs#L25) and they correctly pass serialization options to the serializer (ExecutablePlugin.cs#L31, ExecutablePlugin.cs#L38).

@VictoriousRaptor VictoriousRaptor merged commit a383856 into Flow-Launcher:dev Feb 11, 2024
3 checks passed
@jjw24 jjw24 mentioned this pull request Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working High Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Node.js-based plugins don't work
3 participants