Skip to content

Conversation

@savetheclocktower
Copy link

Creating this as a draft because it's largely duplicated work and I don't want it to overshadow #6.

The issue reported in pulsar-edit#1236 can only be fixed by upgrading one of scandal’s dependencies; and I figured if we touched scandal at all, it would be irresponsible not to decaffeinate it.

I should've checked whether a decaffeination was already in progress!

This PR contains my own decaffeination attempt, but also bumps the dependency in question isbinaryfile to version 3 so that its package.json will no longer be malformed, hence will no longer generate error spam in the console.

If #6 makes it to master, this diff will get a lot smaller, and perhaps it'll be easier to reduce it to just the part that matters (the dependency bump). When I can pull that off, I'll take this out of draft.

@savetheclocktower savetheclocktower mentioned this pull request Jul 27, 2025
@confused-Techie
Copy link
Member

I've gone ahead and merged #6 with your review, so hopefully it's not too much trouble to rework this PR so we can get it merged. Especially exciting to have spec decaf on this PR as well

@savetheclocktower
Copy link
Author

savetheclocktower commented Sep 1, 2025

OK, reworked this PR to be more targeted:

  • Bumps isbinaryfile to v3
  • Converted the specs from CoffeeScript to JS
  • Removed the coffeelint dependency
  • Rewrote the code examples from the README in JS

@savetheclocktower savetheclocktower marked this pull request as ready for review September 1, 2025 21:08
@savetheclocktower
Copy link
Author

Also just added the change described in #1 because it's been open for 2+ years.

Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally hoping to review the spec decafe more closely, but with a quick glance it looks good and tests are still running just fine. As for the changes made in the actual code they look good and are simple enough. I'll go ahead and merge this one.

Although if we are bundling up changes from that other PR, I'll also just go ahead and merge that one then this one so we don't just have to close that out.

EDIT:

Actually realized that other one can't be merged now with our switch to JS. So nvm on that front

@confused-Techie confused-Techie merged commit 3712862 into pulsar-edit:master Sep 6, 2025
1 check passed
@savetheclocktower savetheclocktower deleted the decaffeinate branch September 6, 2025 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants