Skip to content

Support linebroken import lists #788

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

Closed
bluss opened this issue Jan 29, 2016 · 5 comments
Closed

Support linebroken import lists #788

bluss opened this issue Jan 29, 2016 · 5 comments

Comments

@bluss
Copy link
Member

bluss commented Jan 29, 2016

rustfmt wanted to make this change (and many larger, just like this). I'd like to have an option to _not do_ this, to preserve the list formatting, with trailing comma, so that the code is easy to read and edit.

@@ -1,22 +1,19 @@
 use std::fmt;
-use super::{
-    ArrayBase,
-    Data,
-    Dimension,
-};
+use super::{ArrayBase, Data, Dimension};


@@ -87,14 +80,8 @@ pub use stride_error::StrideError;
 pub use si::{Si, S};

 use iterators::Baseiter;
-pub use iterators::{
-    InnerIter,
-    InnerIterMut,
-    OuterIter,
-    OuterIterMut,
-    AxisChunksIter,
-    AxisChunksIterMut,
-};
+pub use iterators::{InnerIter, InnerIterMut, OuterIter, OuterIterMut,
+                    AxisChunksIter, AxisChunksIterMut};

If you're curious, these are my settings:

ideal_width = 80
max_width = 85
fn_call_width = 80
fn_return_indent = "WithWhereClause"
fn_args_density = "Compressed"
normalise_comments = false
where_density = "Tall"
@rnewman
Copy link

rnewman commented Apr 27, 2017

This alone is a huge part of why we don't use rustfmt on our team.

@behnam
Copy link

behnam commented Jun 30, 2017

My comment from a dup issue:

It's a common style to have import names in individual lines, in "Block" style. It's my personal preference, as well, since it allows faster lookup with eyes and easier addition/removal of items with small diff footprint.

Example:

use freetype::freetype::{
    FT_Face,
    FT_Get_Char_Index,
    FT_GlyphSlot,
    FT_Init_FreeType,
    FT_LOAD_DEFAULT,
    FT_Library,
    FT_Load_Glyph,
    FT_New_Face,
    FT_Render_Glyph,
    FT_Render_Mode,
    FT_Set_Pixel_Sizes,
};

Looking at other options with "Block" variant, I think the name of this option should be imported_names_style or imported_names_layout. (I don't see how layout and style options are different, so please advise.)

I think I can take a shot at implementing it myself. Is there a process to approve options before getting to implementation? (I didn't see anything on this in Contributing.md.)

@behnam
Copy link

behnam commented Jun 30, 2017

Looking at the code now, and looks like ListTactic, used also for the where_layout option, is the best type to use for this option, with Vertical value giving us what's asked in this issue, Mixed being the current behavior, and more options to choose from.

pub enum ListTactic {
    // One item per row.
    Vertical,
    // All items on one row.
    Horizontal,
    // Try Horizontal layout, if that fails then vertical.
    HorizontalVertical,
    // HorizontalVertical with a soft limit of n characters.
    LimitedHorizontalVertical(usize),
    // Pack as many items as possible per row over (possibly) many rows.
    Mixed,
}

@RazrFalcon
Copy link

This is the main reason why I do not use rustfmt...

@topecongiro
Copy link
Contributor

rustfmt has imports_layout and imports_indent options to solve this issue (via #1785).
Currently these options are only available on the latest master branch. Please try!

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

No branches or pull requests

5 participants