Skip to content

[fix] resolve -p short option collision between --proxy and --password#373

Merged
vxbinaca merged 1 commit into
bibanon:masterfrom
75ohmantenna:fix/proxy-short-option
May 7, 2026
Merged

[fix] resolve -p short option collision between --proxy and --password#373
vxbinaca merged 1 commit into
bibanon:masterfrom
75ohmantenna:fix/proxy-short-option

Conversation

@75ohmantenna

Copy link
Copy Markdown
Contributor

Summary

The Options section in the CLI docstring assigned -p to both
--proxy and --password. Docopt resolves the conflict by taking the
first match, so -p <val> silently mapped to --proxy and the -p
alias for --password never worked.

This PR moves --proxy to -x, following the curl convention
(curl -x / curl --proxy), which frees -p for exclusive use as the
--password alias.

Changes

  • Change --proxy short option from -p to -x in the CLI docstring.
  • Update the README usage block to match.
  • Add parser tests verifying -x sets --proxy and -p sets
    --password (and not --proxy), guarding against regression.

Breaking change

Scripts or shell aliases that passed -p <proxy-url> for proxy must
switch to -x <proxy-url> or the long form --proxy <proxy-url>.

Testing

pytest tests/ -q
flake8

Test upload

The fix does not affect functionality of TubeUp:
https://archive.org/details/youtube-1CIMGTO6aFc
https://archive.org/details/youtube-HaQiFAWxDxM

The Options section assigned -p to both --proxy and --password.
Docopt resolves the conflict by taking the first match, so -p <val>
silently mapped to --proxy and the -p alias for --password never
worked.

Move --proxy to -x, following the curl convention (curl -x/--proxy),
which frees -p for exclusive use as the --password alias. Update the
README usage block to match. Add parser tests covering both -x and
-p to guard against regression.

Note: scripts passing -p <proxy-url> must switch to -x or --proxy.

Co-authored-by: Codex (gpt-5.5) <codex@openai.com>
@vxbinaca

vxbinaca commented May 6, 2026

Copy link
Copy Markdown
Collaborator

I'm not opposed to merging this however I must ask why this is needed? You're asking us to break old functionality.

Now, my most important user, IA themselves, this wouldn't effect them because they don't need a proxy to upload, what I'm concerned about is power users who use those options. It would introduce a break in their scripts.

Another thing sort of related to this PR is I want to come back to eliminating short flags. Perhaps for now, use --password as a new alternate option so it's safe later on to kill short flags?

Nice work on test uploads it wasn't needed, but thanks.

@brandongalbraith

Copy link
Copy Markdown
Collaborator

@vxbinaca I like this change from a long term maintainability and usability perspective, but I agree with your points about existing scripting functionality. Perhaps we can institute a "brownout" and let folks know in the next release by raising a warning if a proxy URL is used with the -p flag, with the release after being the breaking change. Thoughts?

@vxbinaca

vxbinaca commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Yeah. Next release let's squash short flags. Or by August.

@Andres9890

Copy link
Copy Markdown
Contributor

@vxbinaca

vxbinaca commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Right but the code is sound, I just don't like the approach. Let's remove short flags it's more consistent. We've adopted longer flags for --use-download-archive and --ignore-exisiting-item let's rip the band-aid off. Shorthand flags were the old way of doing things. It makes the flags more readable for issues.

So --proxy for a Proxy, and --user for Username and --pass for Password.

Edit: And his problem that caused the PR, it's real. The flags collide. I know why I didn't notice this, because I prefer to use cookies and don't use a proxy.

@vxbinaca

vxbinaca commented May 7, 2026

Copy link
Copy Markdown
Collaborator

I'm merging this and cutting a new version for the time being but I want to circle back to killing off short flags.

@vxbinaca vxbinaca merged commit e9bfa37 into bibanon:master May 7, 2026
6 checks passed
@brandongalbraith

Copy link
Copy Markdown
Collaborator

Thanks @vxbinaca!

@75ohmantenna 75ohmantenna deleted the fix/proxy-short-option branch May 7, 2026 20:23
@75ohmantenna

Copy link
Copy Markdown
Contributor Author

I'm pretty sure this was made using AI

I should have included a disclaimer in the PR, my apologies. This was my first contribution to a project and I should have mentioned it. On second thought, I would have been better off opening a bug report to discuss it. I'll do better in the future.

@vxbinaca

vxbinaca commented May 7, 2026

Copy link
Copy Markdown
Collaborator

I don't think I'll be accepting any more AI commits, because they have questionable copyright.

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.

4 participants