Skip to content

Conversation

@barosl
Copy link
Contributor

@barosl barosl commented Nov 19, 2014

This pull request feels a bit bikeshedding, but I think it is necessary.

  • os::make_absolute() -> os::abspath(): The name "make_absolute" is too generous to hold a specific meaning. At least it should contain "path". It seems that "abspath" is a common and concise name. (Also used in Python.)
  • os::change_dir() -> os::chdir(): The method names in the os module generally follow the POSIX-like convention, such as setenv, getenv, getcwd, and errno. As os::change_dir() internally uses libc::chdir() on Unix, it would be a good idea to follow the same convention.

The name "make_absolute" is too generous to hold a specific meaning. At
least it should contain "path". It seems that "abspath" is a common and
concise name.

[breaking-change]
The method names in the os module generally follow the POSIX-like
convention, such as setenv, getenv, getcwd, and errno. As
os::change_dir() internally uses libc::chdir() on Unix, it would be a
good idea to follow the same convention.

[breaking-change]
@alexcrichton
Copy link
Member

I know that @aturon has been focused on std::io recently, and it's likely that we're going to move away from the posix-based names in general (like chdir) to more descriptive names which don't bias one platform over another. For that reason, this may want to hold off on the change_dir => chdir rename for now.

Additionally, the make_absolute => abspath change seems related to rust-lang/rfcs#474 so you may want to coordinate with @aturon on that as well.

@michaelsproul
Copy link
Contributor

+1 for Alex's comment. I think internal naming consistency is more desirable than following Python/Unix convention.

@barosl
Copy link
Contributor Author

barosl commented Nov 19, 2014

I'm just a bit confused of dealing with the different conventions at the same time. If we can use the descriptive names for other methods in the os module as well, I don't care. (Such like setenv() -> set_env_var()?)

But I also think that if we are to unify the names into the descriptive fashion, the required changes should be larger than the reverse. So I think sticking to POSIX is still a viable option.

One thing to note, which is slightly (not?) related to this problem is, that the reason why make_absolute() is in os is that it depends on os::getcwd(). I'm not sure this is a valid design, because to me, it seems to be natural that the method is part of Path. Maybe we can discuss this at rust-lang/rfcs#474.

@alexcrichton
Copy link
Member

Yes there are definitely conflicting conventions throughout std::io::fs and std::os, but these are slated to be dealt with when the modules are considered for API stabilization. The POSIX naming convention is definitely a viable option, but we need to have an overall vision for the entire set of I/O apis instead of locally optimizing one module only to maybe have it change radically later.

I also agree that os::make_absolute is odd when I would expect it in the path module!

@bstrie
Copy link
Contributor

bstrie commented Nov 20, 2014

Does @aturon have a formal RFC for naming conventions like this yet?

@alexcrichton
Copy link
Member

Not that I know of, but I'm sure it's coming down the pike!

@alexcrichton
Copy link
Member

Closing due to inactivity, but as an update @aturon and I gave a close look to the std::os module this past work week and an RFC/PR should be coming soon!

lnicola pushed a commit to lnicola/rust that referenced this pull request Feb 17, 2025
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