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

Added processing Tilde (~) sign in filename:absname #825

Closed
wants to merge 1 commit into from
Closed

Added processing Tilde (~) sign in filename:absname #825

wants to merge 1 commit into from

Conversation

dieu
Copy link

@dieu dieu commented Sep 7, 2015

absname(/foo) -> path_to_current_home/foo
absname(
/../x) -> path_to_current_home/../x
absname(~/) -> path_to_current_home

absname(~/foo) -> path_to_current_home/foo
absname(~/../x) -> path_to_current_home/../x
absname(~/) -> path_to_current_home
@mikpe
Copy link
Contributor

mikpe commented Sep 9, 2015

Strongly disagree. This changes absname in an incompatible way. If you want this functionality, it should be in a convenience layer above the filename module, or as a convenience add-on to that module.

@dieu
Copy link
Author

dieu commented Sep 9, 2015

@mikpe thank for your feedback. Can you explain where exactly are incompatible this changes?

And how you think it should be implemented?

@mikpe
Copy link
Contributor

mikpe commented Sep 10, 2015

Tilde is a perfectly valid character in a UNIX file name, and you're changing the behaviour for file names starting with tilde. That's by definition incompatible.

Just put this new behaviour in a new function, then there's no compatibility issue.

@OTP-Maintainer
Copy link

Patch has passed first testings and has been assigned to be reviewed


I am a script, I am not human


@bjorng
Copy link
Contributor

bjorng commented Nov 18, 2015

I agree with @mikpe that the change is incompatible and not acceptable.

@bjorng bjorng closed this Nov 18, 2015
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.

None yet

4 participants