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

add videos to playlist from chat #316

Closed
wants to merge 0 commits into from

Conversation

csandras05
Copy link
Contributor

(Partially) solves issue #262 - adding videos to playlist from terminal (without gui) is still not possible.

@Et0h Et0h self-requested a review May 26, 2020 20:44
@Et0h
Copy link
Contributor

Et0h commented May 26, 2020

Thanks for the PR.

It might be better if users could add any file to the playlist (as users can when you edit the playlist), which could be useful if they are still downloading the file they were hoping to play.

I've not tested it much, but I think this can be done by making the following changes:

  • Remove the various checks in elif command.group('command') in constants.COMMANDS_QUEUE: (within consoleUI.py) and simply using self._syncplayClient.ui.addFileToPlaylist(filename).
  • In def addFileToPlaylist(self, filePath, index=-1): in gui.py, replace if os.path.isfile(filePath): with if not isURL(filePath): and replace elif isURL(filePath): with else:

Note: The check in elif command.group('command') in constants.COMMANDS_QUEUE: for if the parameter is blank could potentially be retained, but the other commands do not have such checks so perhaps you should either skip this error or introduce a generic error for a lack of parameter in the other languages for all of the commands for consistency.

Some language issues:

  • "No file/url given" - All messages should be in messages_en.py with stubs in other languages (with #TODO: Translate). URL should be capitalised. That said, as above, this error might not want to be retained.
  • "\tq [file/url] - add file or url to bottom of playlist", - Should have stubs in other languages. URL should be capitalised.

@Et0h Et0h removed their request for review May 26, 2020 22:14
@csandras05 csandras05 closed this Jun 2, 2020
@odrling odrling mentioned this pull request Aug 7, 2020
Et0h pushed a commit that referenced this pull request Sep 13, 2020
* add videos to playlist from chat

* add urls to playlist

* add files in media directory to playlist

* add commands to show the playlist and select an index

* add command to delete files from the playlist

* show selected index in playlist

* fix adding files with queue command in GUI mode

* start indexing the playlist at 1

or at least that's what it would look like to the user

* start all commands related to playlist with `q`

Co-authored-by: kiscs <csandras05@gmail.com>
albertosottile pushed a commit to albertosottile/syncplay that referenced this pull request Sep 30, 2020
…and Syncplay#319

* add videos to playlist from chat

* add urls to playlist

* add files in media directory to playlist

* add commands to show the playlist and select an index

* add command to delete files from the playlist

* show selected index in playlist

* fix adding files with queue command in GUI mode

* start indexing the playlist at 1

or at least that's what it would look like to the user

* start all commands related to playlist with `q`

Co-authored-by: kiscs <csandras05@gmail.com>
albertosottile pushed a commit to albertosottile/syncplay that referenced this pull request Sep 30, 2020
…and Syncplay#319

* add videos to playlist from chat

* add urls to playlist

* add files in media directory to playlist

* add commands to show the playlist and select an index

* add command to delete files from the playlist

* show selected index in playlist

* fix adding files with queue command in GUI mode

* start indexing the playlist at 1

or at least that's what it would look like to the user

* start all commands related to playlist with `q`

Co-authored-by: kiscs <csandras05@gmail.com>
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