Suggestions about xkcd-dl

[tasdikrahman]

Good work @BlitzKraft 👍

[tasdikrahman]

Thanks a lot for your time. Will fix these issues as soon as possible. 👍

[Kickball]

I have attempted to solve requests 1 and 2 (partially, images are downloaded to working dir) in my pull request #9.

[Kickball]

Now that #9 has been merged most of the changes requested in sections 1 and 2 are implemented.

I'm not up for tackling section 3 or the parameter change in section 2, that'll have to be up to either @prodicus or @bradenbest.

[tasdikrahman]

Update: Both enhancements, 1 and 2 were added by @Kickball in pull request #9 .

Nicely done @Kickball . 👍 😄

[Kickball]

Enhancement 3 can be obtained by merely adding this to line 106:

img_tooltip = img_tag.get('title')

however writing that information to the json file will prove harder.

[tasdikrahman]

Are you referring to the special character's issue when writing to the file @Kickball ?

[bradenbest]

@Kickball I don't see why you can't just write it to a text file called title.txt. In fact, I'll get on that

[bradenbest]

So I found the linkaround 106, and changed a little bit:

                    for data in image_page_content_soup.find_all("div", {"id": "comic"}):
                        for img_tag in data.find_all('img'):
                            img_link = img_tag.get('src')
                            img_tooltip = img_tag.get('title')
                            # here

                    complete_img_url = "http:{url}".format(url=img_link)
                    img_tooltip_file_name = "{current_directory}/xkcd_archive/{name}/title.txt".format(current_directory=WORKING_DIRECTORY, name=xkcd_number)
                    # here

                    file_name = "{description}.jpg".format(description=new_description)
                    urllib.request.urlretrieve(complete_img_url, file_name)
                    write_tooltip(img_tooltip, img_tooltip_file_name)
                    # here

And added the write_tooltip function:

def write_tooltip(tooltip, file_name):
    f = open(file_name, "w")
    f.write(tooltip)
    print("Debug: I should be writing \"%s\" to file '%s'" % ("Hello world", file_name))
    f.close()

Well, that's what it looked like just before I found out why it wasn't writing the file. Yeah, turns out the function isn't even being called.

See, I thought I was editing the function that downloads an xkcd page and saves it to its directory. Turns out it's a function that downloads an xkcd page and saves it to its directory, out of like 3 or 4 other ones.

I was gonna let the 80-line function slide and just say you needed to refactor your code some, but this is ridiculous. This is a classic case of data duplication. Why isn't the downloading abstracted to a single function that takes an argument (the "xkcd_number"), and downloads it? Then you can use that with all of the other xkcd-downloading functions. For example, download_all() loops through all the xkcd_numbers it knows about and calls download_page() with each of them, whereas download_latest() does whatever it does and calls download_page() for each result, and the one that acts on --download=<number> just calls download_page() with the user's input.

Your code needs some cleanup. And I don't even need proof, because the proof is in the code comments: their existence proves that the code is messy.

I'm gonna post that as a separate issue, and when that issue is resolved, I'll make another attempt at implementing enhancement 3

[bradenbest]

And just to be absolutely clear, what, in particular, I am expecting with enhancement 3 is that when you download an image, in addition to <description>.png and description.txt, there should be a file title.txt, that contains the title text of an image, e.g. for #1000, the contents of title.txt should be Thank you for making me feel less alone.

[BlitzKraft]

@bradenbest Title text download is now possible and is covered by f2bdb48

The hover text is saved in the description.txt file. I personally don't like having too many files.

[BlitzKraft]

@prodicus Everything in this issue has been addressed. Please close this.

[tasdikrahman]

PR #23 and the previous PR's implement all the above suggestions.

However if anything is missed, feel free to comment on this issue and I will open it again