Skip to content

Conversation

@Stypox
Copy link
Member

@Stypox Stypox commented Oct 3, 2025

This PR does these things:

  • it reverts all of our commits related to parsing custom JSON format. You can see all of our commits from here:
    • | 72c8ed4 Revert "Support JS-like JSON"
    • | 41f29d8 Revert "Support keys starting with a non-ASCII character"
    • | d3b1b70 Revert "Support single quote strings"
    • | f9d8d68 Revert "Support keywords as keys"
    • | e2cd3f0 Revert "Fix some more semi-string parsing bugs"
    • | 714bb89 Revert "Disallow semi-strings as value"
    • | b52e21a Revert "Fix Checkstyle errors"
    • | 9718d33 Revert "Fix issue for semi-strings as well"
  • the only two commits that were not reverted are 0e64c2b d5da581, which are both also present in FireMasterK's fork
  • additionally, the README changes were retained (5494f3c) and updated with fcb2efd
  • pulls in updates from upstream
  • pulls in @FireMasterK's performance changes (note that this required reverting the other commits, since @FireMasterK's fork has just removed those commits directly, but I would prefer not to force-push on our master branch).
  • I skipped commit c624b79 since it pulls in the fastutil library to squeeze out the last bits of performance, but we'd prefer not to increase APK size, as suggested by @FireMasterK in Use Piped's fork of nanojson NewPipeExtractor#981 (comment), also see TeamNewPipe/NewPipeExtractor@325c07d (#981)
  • I did include commit 24eb336 though, even though it adds the fastdoubleparser library. I don't think it matters much whether we keep it or not, the JAR is just 191kB but I'm pretty sure most of it would be optimized out or merged during build. Let me know if I should revert this.
  • I also skipped commit a507525 and instead made sure we retain compatibility with JDK 11 with 2a27a17.
  • fixes checkstyle

All tests pass. This PR is basically a replacement for TeamNewPipe/NewPipeExtractor#981, where many people suggested updating our own fork. While I initially was against, I now think it's necessary if we want to have some of @FireMasterK's commits but not all.

Closes #2

dependabot bot and others added 30 commits February 11, 2022 23:11
Bumps [maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.3.0 to 3.3.2.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.3.0...maven-javadoc-plugin-3.3.2)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-site-plugin](https://github.com/apache/maven-site-plugin) from 3.9.1 to 3.11.0.
- [Release notes](https://github.com/apache/maven-site-plugin/releases)
- [Commits](apache/maven-site-plugin@maven-site-plugin-3.9.1...maven-site-plugin-3.11.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-site-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps nexus-staging-maven-plugin from 1.6.8 to 1.6.12.

---
updated-dependencies:
- dependency-name: org.sonatype.plugins:nexus-staging-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-compiler-plugin](https://github.com/apache/maven-compiler-plugin) from 3.8.1 to 3.10.1.
- [Release notes](https://github.com/apache/maven-compiler-plugin/releases)
- [Commits](apache/maven-compiler-plugin@maven-compiler-plugin-3.8.1...maven-compiler-plugin-3.10.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-compiler-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…che.maven.plugins-maven-compiler-plugin-3.10.1

Bump maven-compiler-plugin from 3.8.1 to 3.10.1
…che.maven.plugins-maven-javadoc-plugin-3.3.2

Bump maven-javadoc-plugin from 3.3.0 to 3.3.2
…che.maven.plugins-maven-site-plugin-3.11.0

Bump maven-site-plugin from 3.9.1 to 3.11.0
…atype.plugins-nexus-staging-maven-plugin-1.6.12

Bump nexus-staging-maven-plugin from 1.6.8 to 1.6.12
Bumps [maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.3.2 to 3.4.0.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.3.2...maven-javadoc-plugin-3.4.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-site-plugin](https://github.com/apache/maven-site-plugin) from 3.11.0 to 3.12.0.
- [Release notes](https://github.com/apache/maven-site-plugin/releases)
- [Commits](apache/maven-site-plugin@maven-site-plugin-3.11.0...maven-site-plugin-3.12.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-site-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps nexus-staging-maven-plugin from 1.6.12 to 1.6.13.

---
updated-dependencies:
- dependency-name: org.sonatype.plugins:nexus-staging-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [exec-maven-plugin](https://github.com/mojohaus/exec-maven-plugin) from 3.0.0 to 3.1.0.
- [Release notes](https://github.com/mojohaus/exec-maven-plugin/releases)
- [Commits](mojohaus/exec-maven-plugin@exec-maven-plugin-3.0.0...exec-maven-plugin-3.1.0)

---
updated-dependencies:
- dependency-name: org.codehaus.mojo:exec-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…ehaus.mojo-exec-maven-plugin-3.1.0

Bump exec-maven-plugin from 3.0.0 to 3.1.0
…atype.plugins-nexus-staging-maven-plugin-1.6.13

Bump nexus-staging-maven-plugin from 1.6.12 to 1.6.13
…che.maven.plugins-maven-javadoc-plugin-3.4.0

Bump maven-javadoc-plugin from 3.3.2 to 3.4.0
…che.maven.plugins-maven-site-plugin-3.12.0

Bump maven-site-plugin from 3.11.0 to 3.12.0
Bumps [maven-site-plugin](https://github.com/apache/maven-site-plugin) from 3.12.0 to 3.12.1.
- [Release notes](https://github.com/apache/maven-site-plugin/releases)
- [Commits](apache/maven-site-plugin@maven-site-plugin-3.12.0...maven-site-plugin-3.12.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-site-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Implements toString on JsonLazyNumber.
…che.maven.plugins-maven-site-plugin-3.12.1

Bump maven-site-plugin from 3.12.0 to 3.12.1
Bumps [maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.4.0 to 3.4.1.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.4.0...maven-javadoc-plugin-3.4.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-checkstyle-plugin](https://github.com/apache/maven-checkstyle-plugin) from 3.1.2 to 3.2.1.
- [Release notes](https://github.com/apache/maven-checkstyle-plugin/releases)
- [Commits](apache/maven-checkstyle-plugin@maven-checkstyle-plugin-3.1.2...maven-checkstyle-plugin-3.2.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-checkstyle-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…che.maven.plugins-maven-checkstyle-plugin-3.2.1

Bump maven-checkstyle-plugin from 3.1.2 to 3.2.1
…che.maven.plugins-maven-javadoc-plugin-3.4.1

Bump maven-javadoc-plugin from 3.4.0 to 3.4.1
Bumps [maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.4.1 to 3.5.0.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.4.1...maven-javadoc-plugin-3.5.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-surefire-report-plugin](https://github.com/apache/maven-surefire) from 2.22.2 to 3.0.0.
- [Release notes](https://github.com/apache/maven-surefire/releases)
- [Commits](apache/maven-surefire@surefire-2.22.2...surefire-3.0.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-surefire-report-plugin
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
…che.maven.plugins-maven-surefire-report-plugin-3.0.0

Bump maven-surefire-report-plugin from 2.22.2 to 3.0.0
…che.maven.plugins-maven-javadoc-plugin-3.5.0

Bump maven-javadoc-plugin from 3.4.1 to 3.5.0
dependabot bot and others added 16 commits August 26, 2025 11:05
…1.3 (mmastrac#146)

Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.11.2 to 3.11.3.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.11.2...maven-javadoc-plugin-3.11.3)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-version: 3.11.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

Bumps [org.junit.jupiter:junit-jupiter](https://github.com/junit-team/junit-framework) from 5.13.3 to 5.13.4.
- [Release notes](https://github.com/junit-team/junit-framework/releases)
- [Commits](junit-team/junit-framework@r5.13.3...r5.13.4)

---
updated-dependencies:
- dependency-name: org.junit.jupiter:junit-jupiter
  dependency-version: 5.13.4
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@FireMasterK
Copy link
Member

Nice, however in NewPipeExtractor you will need to add support for LazyStrings, like this: FireMasterK/NewPipeExtractor@165b459

@TobiGr
Copy link

TobiGr commented Oct 3, 2025

What is the correct review process for this PR?
I think that the changes which are explained in the description sound reasonable. Reviewing the code line by line isn't necessary in my opinion because it has already been reviewed and is running in Piped. I think this is good to go if the tests in /src/test pass.

I did include commit 24eb336 though, even though it adds the fastdoubleparser library. I don't think it matters much whether we keep it or not, the JAR is just 191kB but I'm pretty sure most of it would be optimized out or merged during build. Let me know if I should revert this.

Including this is ok for me. If you want, feel free to compile two JARs and compare their sizes. I'd also assume that most of the lib is going to be optimized away.

@Stypox
Copy link
Member Author

Stypox commented Oct 3, 2025

@TobiGr the difference seems to be just 664 bytes which is totally fine in my opinion:

34626  nanojson-1.11-SNAPSHOT_with_fastdoubleparser.jar
33962  nanojson-1.11-SNAPSHOT_without_fastdoubleparser.jar

Copy link

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Tests pass locally and the commits were chosen reasonably.

@Stypox Stypox force-pushed the revert-our-changes-and-use-firemasterk branch from 3b2ed7e to 2a27a17 Compare October 3, 2025 14:40
@Stypox
Copy link
Member Author

Stypox commented Oct 3, 2025

I removed the commit that updates to Java 17, and pushed another commit for the pom.xml that makes sure that, independently of the JDK used during build, the resulting JAR is compatible with Java 11. There was only one small thing in the whole codebase that was incompatible with Java 11, so I fixed that.

@Stypox Stypox merged commit c7a6c1c into master Oct 4, 2025
1 check passed
@TobiGr TobiGr deleted the revert-our-changes-and-use-firemasterk branch October 4, 2025 14:28
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.

Update tests to allow single quotes for strings and quote-less keys in objects

9 participants