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

Implement time ago parser and improve localization handling #158

Merged
merged 8 commits into from
Nov 15, 2019

Conversation

mauriciocolli
Copy link
Contributor

@mauriciocolli mauriciocolli commented Apr 25, 2019

Used some of the work that @wojcik-online did as the base, finishing the implementation of the time ago parser.

  • Time ago parsing for stream items where the service shows it (search, channel videos list)
  • Handle special cases for languages where the number is not shown
    • Hebrew, for example, shows "לפני שעתיים" for "2 hours ago". We cannot assume that it is 1 for those cases (popular among other languages), as it would be wrong in the bigger time units.
  • Rework the Downloader base implementation, allowing for more advanced things to be done
    • Get the response headers
    • Response code status and message
    • Possibility of making other type of requests (OPTIONS, HEAD, POST...)
  • Separate the localization from the content country (just like YouTube let's the user choose both).
  • Parse watching count in live streams items
  • Fix parsing of future videos (those with an option to set a reminder)

Edit: So I did see the PR #196 (related TeamNewPipe/NewPipe#2632), and as a result, no language parsing is needed anymore and english will be forced over others.

However, I think the language handling contained in this PR is still useful for NewPipe as we can now force languages easily and, in the future, help parse sites in other languages.

@theScrabi
Copy link
Member

We need review and test on this. Someone willing to help?

Copy link
Contributor

@yausername yausername left a comment

Choose a reason for hiding this comment

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

Looks very good to me!

@chetan00000007

This comment has been minimized.

@mauriciocolli
Copy link
Contributor Author

I'll start to work on this again over the next few days (still have some catch up to do).


So I did see the PR #196 (related TeamNewPipe/NewPipe#2632), and as a result, no language parsing is needed anymore and english will be forced over others.

However, I think the language handling contained in this PR is still useful for NewPipe as we can now force languages easily and, in the future, help parse sites in other languages.

@TobiGr @Stypox @theScrabi

@Stypox
Copy link
Member

Stypox commented Oct 2, 2019

Yes, this is useful, who knows what the future will bring :-)

@TobiGr
Copy link
Member

TobiGr commented Oct 2, 2019

Hey @mauriciocolli, welcome back!

However, I think the language handling contained in this PR is still useful for NewPipe as we can now force languages easily and, in the future, help parse sites in other languages.

Yes, that functionality is a good addition. Please keep it.

wojcik-online and others added 8 commits November 3, 2019 15:46
In the format '2 days ago' (in English) on a YouTube channel page.
(Parser extensible to other pages.)
- Handle special cases for languages where the number is not shown
- Rework the Downloader base implementation, allowing for more
advanced things to be done
- Separate the localization from the content country (just like
YouTube let's the user choose both).
- Make use of the local Maven repository
@TobiGr
Copy link
Member

TobiGr commented Nov 9, 2019

That were a few lines to review :D Looks fantastic to me.

@TobiGr TobiGr merged commit 5c42034 into TeamNewPipe:dev Nov 15, 2019
@mauriciocolli
Copy link
Contributor Author

@TobiGr Oh, the temp commit (6ca4c89) went through. Sorry for the late reply.

@TobiGr
Copy link
Member

TobiGr commented Nov 18, 2019

@mauriciocolli Oh yes. I reverted it in 0c6e2c8

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.

8 participants