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

Move to stable #85

Merged
merged 12 commits into from
Nov 4, 2015
Merged

Move to stable #85

merged 12 commits into from
Nov 4, 2015

Conversation

skade
Copy link
Contributor

@skade skade commented Nov 4, 2015

This introduces multiple changes to make exa work on Rust 1.4.0. Detailed reasoning behind all patches:

  • 679b9e1: The one instance of sum can be trivially replaced by fold(0, Add::add), dropping a convenience feature flag.
  • b3e3825: fs_modes tracking issue currently gives no hope for fs_mode to remain as it is or to be stabilized anytime soon. As the only thing imported is a constant table, this can be copied over into the library - it will most likely break in the future anyways.
  • e8ea96e: The RFC for CString convenience conversions has failed and the corresponding methods are flagged for deprecation. Take the route over String instead, which avoids many of the issues described in the corresponding RFC discussion.
  • 7f53da7: As the "last" element of the Slice is dropped anyways, this can be expressed using the slicing syntax just as well, dropping the need for a feature that only becomes available in Rust 1.5. Add tests to ensure that the API still holds.
  • 7a97b7d: I'm a bit unsure here: I assume that the vector only grows, which allows to use reserve instead of resize, to my reading. Resize will become available in Rust 1.5.

@ogham ogham self-assigned this Nov 4, 2015
@skade
Copy link
Contributor Author

skade commented Nov 4, 2015

Yes. For some reason, 1.3.0 throws this:

   Compiling exa v0.4.0 (file:///Users/skade/Code/rust/exa)
src/output/details.rs:234:17: 280:19 error: invocation of unsafe method requires unsafe function or block [E0133]
src/output/details.rs:234                 scoped.execute(move || {
src/output/details.rs:235                     let mut errors = Vec::new();
src/output/details.rs:236
src/output/details.rs:237                     let mut xattrs = Vec::new();
src/output/details.rs:238                     match file.path.attributes() {
src/output/details.rs:239                         Ok(xs) => {
                          ...
note: in expansion of for loop expansion
src/output/details.rs:229:13: 281:14 note: expansion site
note: in expansion of closure expansion
src/output/details.rs:225:21: 282:10 note: expansion site
src/output/details.rs:234:17: 280:19 help: run `rustc --explain E0133` to see a detailed explanation
error: aborting due to previous error
Could not compile `exa`.

@ogham
Copy link
Owner

ogham commented Nov 4, 2015

1.3.0 fails because the scoped_threadpool crate makes that particular method unsafe for that version to work around a bug in rustc that’s since been fixed. More details here: #84 (comment)

@skade
Copy link
Contributor Author

skade commented Nov 4, 2015

I fixed the tree output by reimplementing resize in terms of a reallocation and a extend.

It's certainly not as efficient as the original, but it works. I added a TODO to keep track that this should be cleaned up after the 1.5.0 release.

@skade
Copy link
Contributor Author

skade commented Nov 4, 2015

Moved the constants to a private module.

@ogham
Copy link
Owner

ogham commented Nov 4, 2015

LGTM! 👍

I’m OK with copy-and-pasting code from libstd in lieu of waiting for some of the features to stabilise, especially if these features are already going to be stabilised soon. The benefit of having everyone able to compile exa themselves outweighs the small cost of ever fixing it if we need to. So thanks very much for this.

ogham added a commit that referenced this pull request Nov 4, 2015
@ogham ogham merged commit caf5fce into ogham:master Nov 4, 2015
sbatial pushed a commit to syphar/zetta that referenced this pull request Jul 30, 2023
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