DrKain / subclean

A cross-platform CLI tool and node module to remove advertising from subtitles. Supports Bazarr and bulk cleaning!
MIT License
54 stars 5 forks source link

Added Node.js Module Support #34

Closed jaruba closed 7 months ago

jaruba commented 10 months ago

I tried my luck at adding node.js module support, there is probably a better way to do this but it does work.

Example usage:

const fs = require('fs');
const SubClean = require("subclean");

const cleanSubs = (srtData) => {
  const cleaner = new SubClean();
  return cleaner.module(srtData, { ne: true });
}

const init = async (srtPath, outputPath) => {
  const srtData = fs.readFileSync(srtPath).toString()
  const srtDataNoAds = await cleanSubs(srtData)
  fs.writeFileSync(outputPath, srtDataNoAds)
}

One disadvantage is that it moves typescript to a dependency (was a devDependency before) because it is needed to do tsc on npm postinstall.

DrKain commented 10 months ago

Hi jaruba, thanks for the PR. I'm currently in the hospital and won't be able to take any action for some time but I can definitely have a look once I'm out.
Sorry for the inconvenience

jaruba commented 10 months ago

@DrKain I'm sad to hear that you have health problems, get well soon!

DrKain commented 8 months ago

Hi @jaruba,

Thanks for the kind wishes. I'm out of the hospital now but still recovering for the most part :) It might be a few more weeks until I'm back on my feet but I've had a look through the code for this PR and for the most part it looks fine, but I wouldn't mind a few changes if you have the time.

I like the idea of passing file data instead of a path but I think it could be tweaked a bit. For example, the noFsOutput variable could be added to the IArguments interface along with the raw input (perhaps with a more fitting name?). File operations could be moved into other functions that check for this variable before attempting to read/write to the file. This would keep things organized and allow more flexibility in the future. Of course, this would still rely on the require.main === module check you have at the end.

I'm totally fine with TypeScript being a dependency. From the data I've collected, most users seem to be using the binaries, and the handful of people using npm wouldn't mind or even notice.

For the most part, I'm done with the core code (with the exception of the module support) other than a few bug fixes or optimizations, but I do plan on a total rework of the filters files and parsing. Using JSON was me being lazy and it limits the regex support. I'm likely to add a custom parser similar to what uBlock Origin uses but a simplified version for this project. I didn't expect this many users in the first place but I'm flattered that I've been able to help so many people.

If you don't have time or you're unwilling to make the changes, just let me know, and I'll merge into the dev branch and tackle them when I'm fully recovered :)

Thanks again, and have a Merry Christmas!

DrKain commented 7 months ago

Hi @jaruba. I've added node module support in the latest version. I've credited you as a contributor in the latest release for your help with the core code