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

Draft: feat: alias support prefix #442

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Draft: feat: alias support prefix #442

wants to merge 1 commit into from

Conversation

bbtfr
Copy link
Member

@bbtfr bbtfr commented Dec 7, 2024

支持一下在 alias 里写 prefix,方便给一些常用的 sftp host 加个 alias

Copy link

codecov bot commented Dec 7, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.70%. Comparing base (81d9691) to head (4fe2cb3).

Files with missing lines Patch % Lines
megfile/cli.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #442      +/-   ##
==========================================
- Coverage   95.73%   95.70%   -0.03%     
==========================================
  Files          44       44              
  Lines        6278     6282       +4     
==========================================
+ Hits         6010     6012       +2     
- Misses        268      270       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bbtfr bbtfr force-pushed the liyang/alias-prefix branch from d338461 to 4fe2cb3 Compare December 7, 2024 09:00
protocol = aliases[protocol]["protocol"]
path = "%s://%s" % (protocol, path_without_protocol)
path = "%s://%s%s" % (protocol, prefix, path_without_protocol)
Copy link
Collaborator

Choose a reason for hiding this comment

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

感觉这个 prefix 可能会引发客服问题,用户可能不知道后面留几个斜杠,不同的协议可能还不一样,比如有人填的是 sftp://ubuntu@host ,正确的应该填 sftp://ubuntu@host// 吧,还有相对路径和绝对路径不知道有没有影响,🤣

Copy link
Member Author

@bbtfr bbtfr Dec 11, 2024

Choose a reason for hiding this comment

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

我自己写也容易写错,但感觉这个并不是 alias 引发的,是 sftp 咱可能定义的太奇怪了
偷偷帮用户补一个 / 感觉也容易有坑,所以我就 bypass 直接传了

你觉得这里应该用 join 吗?比如没写 / 自动补一个?

  • sftp://ubuntu@host = sftp://ubuntu@host/
  • sftp://ubuntu@host/ = sftp://ubuntu@host/
  • sftp://ubuntu@host// = sftp://ubuntu@host//

Copy link
Collaborator

Choose a reason for hiding this comment

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

搞个 sftp config 文件会不会好点

Copy link
Member Author

Choose a reason for hiding this comment

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

先标成 draft 吧,再想想咋整

@bbtfr bbtfr marked this pull request as draft December 12, 2024 02:25
@bbtfr bbtfr changed the title feat: alias support prefix Draft: feat: alias support prefix Dec 12, 2024
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