Skip to content
This repository has been archived by the owner on Nov 12, 2023. It is now read-only.

Use explicit unused bytes instead of inferred for most padding #96

Closed
Antikyth opened this issue Jan 25, 2023 · 0 comments
Closed

Use explicit unused bytes instead of inferred for most padding #96

Antikyth opened this issue Jan 25, 2023 · 0 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested task An essential task for a planned milestone.

Comments

@Antikyth
Copy link
Collaborator

I think as a general code style rule, it is clearer to write:

#[derive(Debug, Hash, PartialEq, Eq, X11Size, Readable, Writable)]
pub struct SetFontPath: Request(51, error::Value) {
    #[allow(clippy::cast_possible_truncation)]
    let path_len: u16 = path => path.len() as u16,
    [_; 2],

    #[context(path_len => usize::from(*path_len))]
    pub path: Vec<LengthString8>,
    [_; path => pad(path)],
}

rather than:

#[derive(Debug, Hash, PartialEq, Eq, X11Size, Readable, Writable)]
pub struct SetFontPath: Request(51, error::Value) {
    #[allow(clippy::cast_possible_truncation)]
    let path_len: u16 = path => path.len() as u16,
    [_; 2],

    #[context(path_len => usize::from(*path_len))]
    pub path: Vec<LengthString8>,
    [_; ..],
}

I asked for some feedback for this from friends, and the first was strongly preferred for clarity and readability.

Unresolved questions

Should unused bytes for filling up to the minimum message limits be explicit, or stay as inferred? I'm leaning towards inferred, but feel free to discuss.

Inferred

pub struct GetSelectionOwner: Reply for request::GetSelectionOwner {
	#[sequence]
	#[derivative(Hash = "ignore", PartialEq = "ignore")]
	pub sequence: u16,

	pub owner: Option<Window>,
	[_; ..],
}

Explicit

pub struct GetSelectionOwner: Reply for request::GetSelectionOwner {
	#[sequence]
	#[derivative(Hash = "ignore", PartialEq = "ignore")]
	pub sequence: u16,

	pub owner: Option<Window>,
	[_; 20],
}
@Antikyth Antikyth added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers question Further information is requested task An essential task for a planned milestone. labels Jan 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested task An essential task for a planned milestone.
Projects
Archived in project
Development

No branches or pull requests

1 participant