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

Fix duplicate download IDs when using "/url save" #1795

Merged
merged 1 commit into from
Mar 10, 2023
Merged

Fix duplicate download IDs when using "/url save" #1795

merged 1 commit into from
Mar 10, 2023

Conversation

ike08
Copy link
Contributor

@ike08 ike08 commented Mar 8, 2023

Fixes #1794

Explanation
The problem is the download's identifier. Downloads are given an ID so they can be referenced later when their progress changes. Currently, the download's ID is the download's URL. When you download the same file twice, you have two downloads with the same ID. Download progress updates are shown on the first of both downloads with the same ID.

Solution
Change the download's ID from its URL to a random number. There is a new function in cmd_funcs.c named "_url_make_id()" that generates a random number for a download's ID. Several other functions are updated to cope with the new ID format.

src/command/cmd_funcs.c Outdated Show resolved Hide resolved
src/command/cmd_funcs.c Outdated Show resolved Hide resolved
@jubalh jubalh added this to the next milestone Mar 8, 2023
@jubalh
Copy link
Member

jubalh commented Mar 9, 2023

A couple of git things:
You have merge commits in there. They shouldn't be there. You want to rebase on master not merge it into this branch.
Also on some commits you are Your Name. And on others you set your email properly and GH recognizes your login.

So I would suggest to remove the merge commits, squash your two commits into one and set your name/email as you want it to be displayed.

If you don't want to do that I can also cherry pick/squash myself.

Also you changed the coding style somewhere. Which makes our CI complain.
As mentioned in https://github.com/profanity-im/profanity/blob/master/CONTRIBUTING.md clang-format can help with that.

@jubalh jubalh self-requested a review March 9, 2023 07:35
Copy link
Member

@jubalh jubalh left a comment

Choose a reason for hiding this comment

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

See comments

src/config/color.c Outdated Show resolved Hide resolved
@jubalh
Copy link
Member

jubalh commented Mar 9, 2023

Looks good now. Would you mind to expand the commit message?
Like use what you already have as the title. then have a new line. then paste what you wrote here on github already, your first comment. Which mentions the issue it fixes and explains how/why.
That would be great.

Fixes #1794

Explanation
The problem is the download's identifier. Downloads are given an ID so they can be referenced later when their progress changes. Currently, the download's ID is the download's URL. When you download the same file twice, you have two downloads with the same ID. Download progress updates are shown on the first of both downloads with the same ID.

Solution
Change the download's ID from its URL to a random number. A random ID is generated when get_random_string() is called from cmd_funcs.c. Several other functions are updated to cope with the new ID format.
@ike08
Copy link
Contributor Author

ike08 commented Mar 9, 2023

I think I made all the changes you requested. Let me know if you find anything else that is not right. Thanks

@jubalh jubalh self-requested a review March 10, 2023 06:50
@jubalh jubalh merged commit 96c1c44 into profanity-im:master Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download progress does not update when saving the same file twice
2 participants