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

Docs: What happens when using CLI and *not* passing --save? #198

Open
AndydeCleyre opened this issue Apr 9, 2021 · 4 comments
Open

Docs: What happens when using CLI and *not* passing --save? #198

AndydeCleyre opened this issue Apr 9, 2021 · 4 comments
Labels
good first issue Good issue for a first-time contributor to tackle question

Comments

@AndydeCleyre
Copy link

Describe the bug
Neither the help output nor the stdout/stderr indicate what is done with the lyrics, when --save is not passed.

Expected behavior
I thought maybe the same json produced with --save would be sent (alone) to stdout, or maybe just the plaintext lyrics content.

But more confidently, the --help output should include what is to be expected when --save is not passed, and it should be made more obvious via the --save-less command's stdout/err.

To Reproduce

$ lyricsgenius song "$(playerctl metadata title)" "$(playerctl metadata artist)"
Searching for "The Whole Damn Thing" by Those Darlins...
Done.
$ lyricsgenius --help
usage: lyricsgenius [-h] [--save] [--max-songs MAX_SONGS] [-q]
                    {song,artist,album} terms [terms ...]

Download song lyrics from Genius.com

positional arguments:
  {song,artist,album}   Specify whether search is for 'song', 'artist' or
                        'album'.
  terms                 Provide terms for search

optional arguments:
  -h, --help            show this help message and exit
  --save                If specified, saves songs to JSON file
  --max-songs MAX_SONGS
                        Specify number of songs when searching for artist
  -q, --quiet           Turn off the API verbosity

Version info

  • Package version 3.0.0
  • OS: Arch Linux

Thanks so much for this project!

@allerter
Copy link
Contributor

It would make sense that not passing --save would send the results to stdout.

@allerter
Copy link
Contributor

As to what happens when you don't pass --save, nothing! The song is fetched and it ends there.

@allerter allerter added question good first issue Good issue for a first-time contributor to tackle labels Apr 10, 2021
@fcagnola
Copy link

Hi!
I noticed that when you search for songs on the CLI nothing is returned as you said, but when searching for an artist their top song titles are returned until the api doesn't find any more songs.
I tried doing this:
Added the last else to the CLI handler to print lyrics in case --save was not specified

    # Handle the command-line inputs
    if args.search_type == "song":
        song = api.search_song(*args.terms)
        if not song:
            if not args.quiet:
                print("Could not find specified song. Check spelling?")
            return
        if args.save:
            if not args.quiet:
                print("Saving lyrics to '{s}'...".format(s=safe_unicode(song.title)))
            song.save_lyrics()
        else:    # new code
            print(song.to_dict()['lyrics'])

I tried doing it and it seems to work.
I haven't really contributed to any open source project before and I don't know exactly how to check if this "else" clause breaks something. I saw there are no detailed contributing guidelines: how would you proceed?
Sorry for the noob question, I just wanted to help and couldn't really understand how

@allerter
Copy link
Contributor

allerter commented Apr 11, 2021

@fcagnola, search_artist only prints the progress of the search. Ultimately when the search is done, it doesn't return anything either.
Currently, we don't have any contributing guidelines, but I'll make sure to add one in the future. If you'd like to contribute to this issue, make the changes you intend and open a pull request. We'll review it there.
Now as to the changes themselves, I'm not sure which way to go here. When there's no --save we could print the lyrics or maybe the whole JSON. Or we could make the commands save to a JSON file by default and --save (or an equivalent) would only act as a path for the JSON file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for a first-time contributor to tackle question
Projects
None yet
Development

No branches or pull requests

3 participants