Extend atoms_struct macro to allow configuring only_if_exists

[acheronfail]

I'm experimenting with creating a Window Manager with this library, but I've found if my WM starts up before any other X windows, then none of the Atoms have been interned in the X server yet, so Atoms::intern_all actually does nothing (all atoms are returned as ATOM_NONE).

I think what I want to do is make my WM intern all the Atoms that don't already exist... It seems a way to do is is to send x::InternAtom with only_if_exists: false. As far as I understand it, this is the "right" way to do this:

if only_if_exists, is set to false, and the string does not exist on the server, then it is going to be stored, and an atom is returned for that string. if the string does exist, then the atom identifying it, is returned.

From https://twiserandom.com/unix/x11/what-is-an-atom-in-x11/index.html

So, I propose this change which adds another method on the generated Atoms struct to allow consumers of this library to force Atom interning should they so desire.

Some notes:

• I struggled with what to name the method, please suggest alternatives if you have any
• I've kept this backwards compatible by maintaining the Atoms::intern_all signature

[rtbo]

This is good, but I'd rather have only_if_exists configurable per atom.

Something like:

xcb::atoms_struct! {
    #[derive(Debug)]
    struct Atoms {
        wm_protocols  => b"WM_PROTOCOLS"                 only_if_exists,
        wm_del_window => b"WM_DELETE_WINDOW"             only_if_exists,
        wm_state      => b"_NET_WM_STATE"                only_if_exists,
        wm_state_maxv => b"_NET_WM_STATE_MAXIMIZED_VERT" only_if_exists,
        wm_state_maxh => b"_NET_WM_STATE_MAXIMIZED_HORZ" only_if_exists,
        clipboard     => b"CLIPBOARD", // this one is has only_if_exists = false
    }
}

The problem with current version of atoms_struct is that only_if_exists defaults to true. So this would not be backward compatible, and atoms_struct is stabilized in version 1.1. Therefore another macro is needed. The simplest that comes to mind is xcb::atoms_struct2.

[acheronfail]

We could keep it backwards compatible if we slightly changed your suggestion to something like this:

xcb::atoms_struct! {
    #[derive(Debug)]
    struct Atoms {
        // these have only_if_exists = true (the 1.1 default)
        wm_protocols  => b"WM_PROTOCOLS",
        wm_del_window => b"WM_DELETE_WINDOW",
        wm_state      => b"_NET_WM_STATE",
        // this one has only_if_exists = true
        wm_state_maxv => b"_NET_WM_STATE_MAXIMIZED_VERT" only_if_exists = true, 
        // this one has only_if_exists = false
        wm_state_maxh => b"_NET_WM_STATE_MAXIMIZED_HORZ" only_if_exists = false,
    }
}

I'm happy with either suggestion though, so let me know which you prefer and I'll update this Pull Request. :slightly_smiling_face:

[acheronfail]

Alright, so I updated this PR and added in a (non-breaking!) feature to the original macro.

It looks like this now:

xcb::atoms_struct! {
    #[derive(Debug)]
    struct Atoms {
        // these have only_if_exists = true (the 1.1 default)
        wm_protocols  => b"WM_PROTOCOLS",
        wm_del_window => b"WM_DELETE_WINDOW",
        wm_state      => b"_NET_WM_STATE",
        // this one has only_if_exists = true
        wm_state_maxv => b"_NET_WM_STATE_MAXIMIZED_VERT"; only_if_exists = true, 
        // this one has only_if_exists = false
        wm_state_maxh => b"_NET_WM_STATE_MAXIMIZED_HORZ"; only_if_exists = false,
    }
}

Note that I had to use ; as a separator, since $name is of type expr, and the only tokens that are allowed to follow an expr are ;, , and => (and we can't use , since it's already used to split each line). I think it's fine, especially since it's completely optional.

[rtbo]

Is it not possible to remove the ; entirely? (separate tokens with a space)

[acheronfail]

Unfortunately not, since the previous token $name is of type expr. If I make it a space, this is the error:

error: `$name:expr` may be followed by `only_if_exists`, which is not allowed for `expr` fragments
   --> src/lib.rs:445:76
    |
445 |                 $(#[$fmeta:meta])* $fvis:vis $field:ident => $name:expr $( only_if_exists = $only_if_exists:expr)?,
    |                                                                            ^^^^^^^^^^^^^^ not allowed after `expr` fragments
    |
    = note: allowed there are: `=>`, `,` or `;`

error: could not compile `xcb` due to previous error

It will work with only a space if we change $name from expr to tt - but, I guess that's technically a breaking change? :thinking:

I've gone ahead and made the change - I'm not 100% convinced it would be a breaking change, but I'll leave that choice up to you. :wink:

[rtbo]

As long as existing code continues to work as expected I guess we're ok regarding semver.

[rtbo]

Sorry I took so long to review. You finally could remove the ;. Thanks a lot