import hell during merges

[ilya-epifanov]

Consider adding rustfmt.toml:

reorder_imports = true
imports_granularity = "Item"
group_imports = "StdExternalCrate" 

This will greatly simplify conflict resolution.

Old:

use std::{collections::HashMap, convert::TryFrom, os::unix::net::UnixStream, sync::Arc};

use crate::{ssd::DecoratedWindow, AnvilState};
use smithay::{
    desktop::{Kind, Space, Window, X11Surface},
    reexports::wayland_server::{protocol::wl_surface::WlSurface, Client, DisplayHandle, Resource},
    utils::{x11rb::X11Source, Logical, Point},
    wayland::compositor::give_role,
};
use x11rb::{
    connection::Connection as _,
    errors::ReplyOrIdError,
    protocol::{
        composite::{ConnectionExt as _, Redirect},
        xproto::{
            ChangeWindowAttributesAux, ConfigWindow, ConfigureWindowAux, ConnectionExt as _, EventMask,
            Window as X11Window, WindowClass,
        },
        Event,
    },
    rust_connection::{DefaultStream, RustConnection},
};

New:

use std::collections::HashMap;
use std::convert::TryFrom;
use std::os::unix::net::UnixStream;
use std::sync::Arc;

use smithay::desktop::Kind;
use smithay::desktop::Space;
use smithay::desktop::Window;
use smithay::desktop::X11Surface;
use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface;
use smithay::reexports::wayland_server::Client;
use smithay::reexports::wayland_server::DisplayHandle;
use smithay::reexports::wayland_server::Resource;
use smithay::utils::x11rb::X11Source;
use smithay::utils::Logical;
use smithay::utils::Point;
use smithay::wayland::compositor::give_role;
use x11rb::connection::Connection as _;
use x11rb::errors::ReplyOrIdError;
use x11rb::protocol::composite::ConnectionExt as _;
use x11rb::protocol::composite::Redirect;
use x11rb::protocol::xproto::ChangeWindowAttributesAux;
use x11rb::protocol::xproto::ConfigWindow;
use x11rb::protocol::xproto::ConfigureWindowAux;
use x11rb::protocol::xproto::ConnectionExt as _;
use x11rb::protocol::xproto::EventMask;
use x11rb::protocol::xproto::Window as X11Window;
use x11rb::protocol::xproto::WindowClass;
use x11rb::protocol::Event;
use x11rb::rust_connection::DefaultStream;
use x11rb::rust_connection::RustConnection;

use crate::AnvilState;

One downside is that it's a nightly feature (I'm using it for a long time though).
cargo +nightly fmt

[i509VCB]

As much as I find the import scheme to be nicer, I'm hesitant to require nightly for formatting as it is easy to forget to format with nightly and the formatting can be thrown out of order sometimes by running with stable and then nightly to fix the problems.

But on the other side of the issue, we can't really do much about rustfmt failing to keep up with the rest of the ecosystem. GATs probably won't get formatting for a while in rustfmt. let else also probably won't get proper formatting either.

Essentially I am not sure what we should do here, @PolyMeilex and @Drakulix may have different opinions on this.

[PolyMeilex]

Nightly is no-go IMO, requiring downloading nightly to contribute is weird, and another thing is we would have to lock the nightly version to certain one, so even if someone already had the nightly formatter, they will need to also download one compatible with our codebase.

[Drakulix]

Yes, honestly I agree with @PolyMeilex here.

To be perfectly honest, I prefer personally the readability of nested imports. I absolutely get that it is bad for merges, but so far (and I have done a bunch of massive PRs) this was never a very huge issue? Might just be me though, so if people disagree I am open to introduce formatting options, as long as it is stable.

[ilya-epifanov]

@PolyMeilex I think it's a shame that this feature is still nightly. I'm using it in all my projects, even those for which I'm the only contributor. BTW I use nightly without pinning the version and I have never had it reorder the imports on me. But I agree having to use nightly channel is a weird requirement.

Anyway I just tried to rebase a branch I was interested in (#726) onto master and was quickly scared away.

[Drakulix]

Anyway I just tried to rebase a branch I was interested in (#726) onto master and was quickly scared away.

That branch mostly was a test for #727 at the time. But given that both that PR and various follow-up PRs have changed the render-element code very significantly, there is no way this branch will rebase cleanly.
Given it was a testing ground in the middle of writing said code, I would guess re-writing it is faster than trying to rebase the work done there. (As such I don't view the import-mess as a big issue here, given the branch has significantly diverged anyway.)

Parts of smithay are moving very fast. In particular the new rendering/damage code was a huge effort over multiple months and is still very fresh. If you are interested in working on this or want some directions about hacking up your own decorations, I strongly suggest joining our matrix chat. We are happy to help anybody venturing into smithay. :)

[ilya-epifanov]

@Drakulix yeah I saw immediately that I just can't rebase it :) Thanks for the pointer. I think I need to spend some time reading https://wayland-book.com/