Skip to content

Stable#691

Merged
Cynede merged 2 commits intouutils:masterfrom
steveklabnik:stable
Sep 28, 2015
Merged

Stable#691
Cynede merged 2 commits intouutils:masterfrom
steveklabnik:stable

Conversation

@steveklabnik
Copy link
Contributor

built on top of #690, this removes some feature gates, in an effort to get closer to stable.

one or two more of these were trickier, so it's not fully on stable yet.

@Cynede Cynede mentioned this pull request Sep 28, 2015
Cynede pushed a commit that referenced this pull request Sep 28, 2015
@Cynede Cynede merged commit a5e6f8f into uutils:master Sep 28, 2015
@Cynede
Copy link
Contributor

Cynede commented Sep 28, 2015

thank you, strange for me why tests are failing...

@ebfe
Copy link
Contributor

ebfe commented Sep 28, 2015

This broke the unexpand tests

running 14 tests
test unexpand_aflag_0 ... ok
test unexpand_aflag_1 ... ok
test unexpand_init_list_0 ... ok
test unexpand_first_only_1 ... ok
test unexpand_first_only_0 ... ok
test unexpand_init_list_1 ... ok
test unexpand_init_1 ... ok
test unexpand_aflag_2 ... ok
test unexpand_init_0 ... ok
test unexpand_trailing_space_0 ... FAILED
test unexpand_spaces_follow_tabs_0 ... FAILED
test unexpand_spaces_follow_tabs_1 ... FAILED
test unexpand_spaces_after_fields ... FAILED
test unexpand_trailing_space_1 ... ok

failures:

---- unexpand_trailing_space_0 stdout ----
        thread 'unexpand_trailing_space_0' panicked at 'assertion failed: `(left == right)` (left: `"123 \t1\n123 1\n123 \n123 "`, right: `"123\t\t1\n123 1\n123 \n123 "`)', /src/uutils/test/unexpand.rs:80


---- unexpand_spaces_follow_tabs_0 stdout ----
        thread 'unexpand_spaces_follow_tabs_0' panicked at 'assertion failed: `(left == right)` (left: `"  \t\t   A"`, right: `"\t\t   A"`)', /src/uutils/test/unexpand.rs:95


---- unexpand_spaces_follow_tabs_1 stdout ----
        thread 'unexpand_spaces_follow_tabs_1' panicked at 'assertion failed: `(left == right)` (left: `"a \t\t\t B \t"`, right: `"a\t\t  B \t"`)', /src/uutils/test/unexpand.rs:107


---- unexpand_spaces_after_fields stdout ----
        thread 'unexpand_spaces_after_fields' panicked at 'assertion failed: `(left == right)` (left: `"   \t\t    A B C D\t\tA\t\n"`, right: `"\t\tA B C D\t\t    A\t\n"`)', /src/uutils/test/unexpand.rs:114



failures:
    unexpand_spaces_after_fields
    unexpand_spaces_follow_tabs_0
    unexpand_spaces_follow_tabs_1
    unexpand_trailing_space_0

test result: FAILED. 10 passed; 4 failed; 0 ignored; 0 measured

Makefile:311: recipe for target 'test_unexpand' failed
make: *** [test_unexpand] Error 101

@Cynede Cynede mentioned this pull request Sep 28, 2015
@ebfe
Copy link
Contributor

ebfe commented Sep 28, 2015

This failure is somewhat hidden because test/common/util.rs fails to compile with latest nightly. Can be fixed with:

diff --git a/test/common/util.rs b/test/common/util.rs
index af86c26..f36d546 100644
--- a/test/common/util.rs
+++ b/test/common/util.rs
@@ -1,7 +1,7 @@
 #![allow(dead_code)]

 use std::env;
-use std::fs::{self, File, PathExt};
+use std::fs::{self, File};
 use std::io::{Read, Write};
 #[cfg(unix)]
 use std::os::unix::fs::symlink as symlink_file;

@Cynede Cynede mentioned this pull request Sep 28, 2015
@Cynede
Copy link
Contributor

Cynede commented Sep 28, 2015

@ebfe I'm trying it: #693

@ebfe
Copy link
Contributor

ebfe commented Sep 28, 2015

The problem is that the new version treats '\t' '\n' as 0 width characters.
cc @steveklabnik

@steveklabnik
Copy link
Contributor Author

Ah ha, this must have been a subtle difference between

let nbytes = UnicodeWidthChar::width(buf[byte] as char).unwrap_or(0)

and

let nbytes = utf8_char_width(buf[byte]);

I'm not sure what the right answer is, exactly, should we just test for \t and \n directly?

Also, how did you get that expanded test output, so I can see for myself?

@ebfe
Copy link
Contributor

ebfe commented Sep 28, 2015

On Mon, Sep 28, 2015 at 07:35:17AM -0700, Steve Klabnik wrote:

Ah ha, this must have been a subtle difference between

let nbytes = UnicodeWidthChar::width(buf[byte] as char).unwrap_or(0)

and

let nbytes = utf8_char_width(buf[byte]);

I'm not sure what the right answer is, exactly, should we just test for \t and \n directly?

Hmm, I only tested \n \t probably other control characters are also
affected.

Also, how did you get that expanded test output, so I can see for myself?

make test or make test BUILD=unexpand

@ebfe
Copy link
Contributor

ebfe commented Sep 28, 2015

Ah ha, this must have been a subtle difference between

let nbytes = UnicodeWidthChar::width(buf[byte] as char).unwrap_or(0)

and

let nbytes = utf8_char_width(buf[byte]);

On a second look I think these 2 are fundamentally different.
utf8_char_width cares about how many bytes are in the utf-8 encoded char
starting with buf[byte] and UnicodeWidthChar::width cares about the
displayed length of the char.

@ebfe ebfe mentioned this pull request Oct 5, 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.

3 participants