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.

Sign up for GitHub

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

Jump to bottom

Fix clippy and tests fail due to simple problems #415

Merged
merged 22 commits into from
Jul 18, 2024
Merged

Fix clippy and tests fail due to simple problems #415

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
97da9e4
Fix clippy
strange-dv Jul 11, 2024
0a6644e
Make warnings go away
strange-dv Jul 12, 2024
efadefc
Make tests alive again (not fully functional)
strange-dv Jul 13, 2024
88f7cf6
Return some tests to the useful state
strange-dv Jul 13, 2024
a41eb54
Fix formatting
strange-dv Jul 13, 2024
df1270a
Fix removed tests
strange-dv Jul 13, 2024
7aece9f
Fix formatting
strange-dv Jul 13, 2024
2086f17
Fix the flag test
strange-dv Jul 13, 2024
ec8e9be
Fix a fix
strange-dv Jul 13, 2024
16cb6e8
Fix for another fix, again
strange-dv Jul 13, 2024
a378138
Fix formatting
strange-dv Jul 13, 2024
a99b27b
Make sure fix will fix the problem
strange-dv Jul 13, 2024
257e58b
fix unicode-width viersion and remevo unused fieild
strange-dv Jul 17, 2024
8554cac
remove one more unused fieild
strange-dv Jul 17, 2024
8ef3a04
fix unparsed ident
strange-dv Jul 17, 2024
ff4c61d
Fix cargo fmt
zhiburt Jul 17, 2024
9c346d4
Update MSRV
zhiburt Jul 18, 2024
5918067
Remove MSRV requirment
zhiburt Jul 18, 2024
35f78f6
Revert unicode_dep && wrap update
zhiburt Jul 18, 2024
8fb764a
cargo clippy
zhiburt Jul 18, 2024
c082733
Merge branch 'master' of github.com:zhiburt/tabled into strangedv-fix…
zhiburt Jul 18, 2024
4c4f28f
fix
zhiburt Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions papergrid/tests/grid/format_configuration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,16 +938,16 @@ fn tabs_arent_handled() {
assert_eq!(
grid.build(),
static_table!(
"+-----------------------+"
"|{ |"
"+-------------------------+"
"|{ |"
"|\t\t \"id\": \"1\", |"
"|\t\t \"name\": \"Hello World\",|"
"|\t\t \"list\": [ |"
"|\t\t\t\t [1, 2, 3], |"
"|\t\t\t\t [4, 5, 6], |"
"|\t\t\t\t [1, 2, 3], |"
"|\t\t\t\t [4, 5, 6], |"
"|\t\t ] |"
"|} |"
"+-----------------------+"
"|} |"
"+-------------------------+"
),
);
}
26 changes: 13 additions & 13 deletions papergrid/tests/grid/render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,31 +210,31 @@ test_table!(
test_table!(
hieroglyph_handling_2,
grid(2, 1).data([["জী._ডি._ব্লক_সল্টলেক_দূর্গা_পুজো_২০১৮.jpg"], ["Hello"]]).build(),
"+------------------------------------+"
"+-----------------------------------+"
"|জী._ডি._ব্লক_সল্টলেক_দূর্গা_পুজো_২০১৮.jpg|"
"+------------------------------------+"
"|Hello |"
"+------------------------------------+"
"+-----------------------------------+"
"|Hello |"
"+-----------------------------------+"
);

test_table!(
doesnt_render_return_carige_0,
grid(2, 2).change_cell((0, 1), "123\r\r\r567").build(),
"+---+------+"
"+---+---------+"
"|0-0|123\r\r\r567|"
"+---+------+"
"|1-0|1-1 |"
"+---+------+"
"+---+---------+"
"|1-0|1-1 |"
"+---+---------+"
);

test_table!(
doesnt_render_return_carige_1,
grid(2, 2).change_cell((1, 1), "12345678").change_cell((0, 1), "123\r\r\r567").build(),
"+---+--------+"
"|0-0|123\r\r\r567 |"
"+---+--------+"
"|1-0|12345678|"
"+---+--------+"
"+---+---------+"
"|0-0|123\r\r\r567|"
"+---+---------+"
"|1-0|12345678 |"
"+---+---------+"
);

// #[test]
Expand Down
6 changes: 3 additions & 3 deletions tabled/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ pub use crate::{tabled::Tabled, tables::Table};
/// }
/// ```
///
/// There's also a probably more sutable way for formatting, if your format is contant.
/// /// There's also a probably more sutable way for formatting, if your format is contant.
/// Using `#[tabled(format = "{}")]` and `#[tabled(format("{}"))]` and proving a general formatting string.
///
/// ```
Expand All @@ -439,8 +439,8 @@ pub use crate::{tabled::Tabled, tables::Table};
/// #[derive(Tabled)]
/// struct Record {
/// #[tabled(skip)]
/// id: [u8; 4],
/// #[tabled(format("{}.{}.{}.{}.{}", self.id[0], self.id[1], self.id[2], self.id[3], self.name)]
/// id: u8,
/// #[tabled(format("{}.{}", self.id, self.name))]
Comment on lines -442 to +443
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do not support formatting using complex expressions like self.id[0] so I simplified this code a bit

Copy link
Owner

@zhiburt zhiburt Jul 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That's actually one of my endless (not released patches) 😄
I did rewrite this support more then a few times.

I wonder what do you think in this regard.
Shall we support functions calls, array index etc?

#[tabled(format("{}.{}", some_function(self.id), self.name.one_more_field[100]))]
id: u8,

Why I did rewrote it is cause it's impossible to support in inline enums 😞
And having 2 different approaches in 2 different places seeeems like not the right approach.

#[derive(Tabled)]
enum Something {
#[tabled(inline)]
A {
   #[tabled(format("{}", self.field1, func(0, 1, self)))]  // <- impossible to complex expressions with self 
   field1: usize,
},
B {}
}

So anyway maybe you have some thoughts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by rewriting? What do we have now if this is not possible to implement for inline structs? Is this the same functionality I assumed for the test fix? If so, I'm sure that supporting more advanced formatting functionality is a good feature, but obviously, there are many complications that can affect the decision of whether it's worth implementing or not.

If you have patches that extend this functionality beyond the scope of the changed test, what should this test look like to test as much of #[tabled(format()] as possible?

/// name: String,
/// }
/// ```
Expand Down
12 changes: 10 additions & 2 deletions tabled/src/settings/width/wrap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,17 @@ fn chunks(s: &str, width: usize, prefix: &str, suffix: &str) -> Vec<String> {
let _ = write!(&mut line, "{}", text_style.start());

while !text_slice.is_empty() {
let available_space = width - line_width;
let available_space = if width > line_width {
width - line_width
} else {
list.push(line);
line = String::with_capacity(width);
line_width = 0;
continue;
};

let part_width = unicode_width::UnicodeWidthStr::width(text_slice);

if part_width <= available_space {
line.push_str(text_slice);
line_width += part_width;
Expand Down Expand Up @@ -1431,7 +1439,7 @@ mod tests {
"\u{1b}[37m(l\u{1b}[39m\u{1b}[37m\u{1b}[41marg\u{1b}[39m\u{1b}[49m\u{1b}[37mest \u{1b}[39m \n",
"\u{1b}[37minteger \u{1b}[39m \n",
"\u{1b}[37mless than \u{1b}[39m\n",
"\u{1b}[37more equal \u{1b}[39m \n",
"\u{1b}[37mor equal \u{1b}[39m \n",
Copy link
Contributor

@Alexdelia Alexdelia Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

that's my fault, sorry folks

Copy link
Contributor

@Alexdelia Alexdelia Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I've re-reviewed my previous fix typos MR to make sure I didn't make other mistakes and I've reverted these error

At the same time, I fix newly found typos

I hope you pardon my mistake and appreciate the MR

#416

"\u{1b}[37mto that \u{1b}[39m \n",
"\u{1b}[37mnumber).\u{1b}[39m ",
)
Expand Down
4 changes: 2 additions & 2 deletions tabled/src/util/string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ mod tests {
assert_eq!(cut_str("🏳️🏳️", 0), "");
assert_eq!(cut_str("🏳️🏳️", 1), "🏳");
assert_eq!(cut_str("🏳️🏳️", 2), "🏳\u{fe0f}🏳");
assert_eq!(string_width("🏳️🏳️"), string_width("🏳\u{fe0f}🏳"));
assert_eq!(string_width("🏳️🏳️"), string_width("🏳️\u{fe0f}🏳️"));

assert_eq!(cut_str("🎓", 1), "�");
assert_eq!(cut_str("🎓", 2), "🎓");
Expand Down Expand Up @@ -258,7 +258,7 @@ mod tests {
);
assert_eq!(
string_width(&emojies),
string_width("\u{1b}[31;100m🏳\u{fe0f}🏳\u{1b}[39m\u{1b}[49m")
string_width("\u{1b}[31;100m🏳️\u{fe0f}🏳️\u{1b}[39m\u{1b}[49m")
);
}

Expand Down
2 changes: 1 addition & 1 deletion tabled/tests/core/extended_table_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ fn record_template() {
let data: Vec<TestType> = vec![TestType, TestType];

let description = "ROW";
let table = ExtendedTable::new(&data)
let table = ExtendedTable::new(data)
.template(move |index| format!("< {} {} >", description, index + 1));
let table = table.to_string();

Expand Down
16 changes: 8 additions & 8 deletions tabled/tests/core/table_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,15 +737,15 @@ mod derived {
},
]
}),
"+------------------------------------+-----------------+-------------------------------+--------+"
"| name | author | text | rating |"
"+------------------------------------+-----------------+-------------------------------+--------+"
"| Rebase vs Merge commit in depth 👋 | Rose Kuphal DVM | A multiline | 43 |"
"| | | text with 🤯 😳 🥵 🥶 | |"
"+------------------------------------+-----------------+--------------------------------+--------+"
"| name | author | text | rating |"
"+------------------------------------+-----------------+--------------------------------+--------+"
"| Rebase vs Merge commit in depth 👋 | Rose Kuphal DVM | A multiline | 43 |"
"| | | text with 🤯 😳 🥵 🥶 | |"
"| | | a bunch of emojies ☄\u{fe0f} 💥 🔥 🌪 | |"
"+------------------------------------+-----------------+-------------------------------+--------+"
"| Keep it simple | Unknown | 🍳 | 100 |"
"+------------------------------------+-----------------+-------------------------------+--------+"
"+------------------------------------+-----------------+--------------------------------+--------+"
"| Keep it simple | Unknown | 🍳 | 100 |"
"+------------------------------------+-----------------+--------------------------------+--------+"
);
}

Expand Down
11 changes: 6 additions & 5 deletions tabled/tests/derive/derive_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ macro_rules! test_tuple {
$($($init_block)*)?

#[derive(Tabled)]
struct TestType(
$( $(#[$attr])* $ty, )*
#[allow(dead_code)]
pub struct TestType(
$( $(#[$attr]) * $ty, )*
);

let value = TestType($($init,)*);
Expand Down Expand Up @@ -244,7 +245,7 @@ mod tuple {
#[test]
fn format3() {
#[derive(Debug, Tabled)]
struct StructName(u8, #[tabled(format("foo {} {:?}", 2, self))] String);
pub struct StructName(u8, #[allow(dead_code)] #[tabled(format("foo {} {:?}", 2, self))] String);

let value = StructName(0, String::from("string"));

Expand Down Expand Up @@ -1055,7 +1056,7 @@ fn rename_all_variants() {
( $name:ident, $case:expr ) => {
#[derive(Tabled)]
#[tabled(rename_all = $case)]
struct $name {
pub struct $name {
field: usize,
}
};
Expand Down Expand Up @@ -1211,7 +1212,7 @@ fn test_reimport_trait_by_crate_attribute() {
#[test]
fn test_display_with_2() {
#[derive(tabled::Tabled)]
struct Struct<'a> {
pub struct Struct<'a> {
#[tabled(display_with("std::path::Path::display"))]
path: &'a std::path::Path,
}
Expand Down
2 changes: 1 addition & 1 deletion tabled/tests/settings/column_names_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ test_table!(
test_table!(
colors_empty,
Table::new([("Hello", "World"), ("and", "looooong\nword")])
.with(ColumnNames::default().color(vec![Color::default(); 0])),
.with(ColumnNames::default().color({ Color::default(); vec![] as std::vec::Vec<tabled::settings::Color> })),
"+&str---+&str------+"
"| Hello | World |"
"+-------+----------+"
Expand Down
20 changes: 10 additions & 10 deletions tabled/tests/settings/render_settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,16 @@ test_table!(
test_table!(
tab_isnot_handled_by_default_test,
Matrix::iter(tab_data1()).with(Style::psql()),
" N | column 0 | column 1 | column 2 "
"--------------+----------+----------+----------"
" 0 | 0-0 | 0-1 | 0-2 "
" 123\t123\tasdasd | 1-0 | 1-1 | 1-2 "
" 2 | 2-0 | htt\tps:// | 2-2 "
" | | www | "
" | | . | "
" | | red\that | "
" | | .c\tom | "
" | | /en | "
" N | column 0 | column 1 | column 2 "
"----------------+----------+-----------+----------"
" 0 | 0-0 | 0-1 | 0-2 "
" 123\t123\tasdasd | 1-0 | 1-1 | 1-2 "
" 2 | 2-0 | htt\tps:// | 2-2 "
" | | www | "
" | | . | "
" | | red\that | "
" | | .c\tom | "
" | | /en | "
);

test_table!(
Expand Down
1 change: 1 addition & 0 deletions tabled_derive/src/parse/field_attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub fn parse_field_attributes(
}

pub struct FieldAttr {
#[allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

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

why choose to allow dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to hear from @zhiburt on this.
I see this field was added in ba048db. I know it was a long time ago, but do you remember why it exists but never read or it's fine if I remove it?

As for those #[allow(dead_code)] that I left in the test code, I think the reason why they are there is more understandable - they are intended for fields that we never read in tests, but should be able to write to them

Copy link
Owner

Choose a reason for hiding this comment

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

As for those #[allow(dead_code)] that I left in the test code, I think the reason why they are there is more understandable - they are intended for fields that we never read in tests, but should be able to write to them

correct;
though these test could be cleaner I guess

if I remove it?

Will be all right.

pub ident: Ident,
pub kind: FieldAttrKind,
}
Expand Down
1 change: 1 addition & 0 deletions tabled_derive/src/parse/type_attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub fn parse_type_attributes(
}

pub struct TypeAttr {
#[allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we also remove this dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Damn, missed it, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

that's what reviews are for

pub ident: Ident,
pub kind: TypeAttrKind,
}
Expand Down