Skip to content

Conversation

@BCSharp
Copy link
Member

@BCSharp BCSharp commented Feb 7, 2025

These functions are added in Python 3.11, but I implemented them now while I have the internals still fresh in my memory.

}

result = PythonFcntl.ioctl(fd, TIOCSWINSZ, buf);
if (ToTermiosError(context, result) is not null and var ex2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer is not null and var ex2 over is object ex2? Anyway, in this case it could just be return ToTermiosError(context, result). But I'm fine with keeping the pattern for consistency...

Copy link
Member Author

Choose a reason for hiding this comment

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

Prefer is not null and var ex2 over is object ex2?

It wasn't a matter of a preference. The form is object simply didn't occur to me. I've checked IL and both cases unsurprisingly generate exactly the same code. So now it is a matter of preference, and I don't have any strong feelings around the choice. The former is more clear what the intention is ("test that it is not null and capture into a variable") but the latter is more concise, which is always a good thing. As for readability of the latter, I can learn a new idiom. Since I have used the former pattern on at least two other occasions, I will merge this PR as is, and change them all together once we establish a decisive preference.

Anyway, in this case it could just be return ToTermiosError(context, result). But I'm fine with keeping the pattern for consistency...

Yes, here I prefer to keep it as is. Not only for consistency with the pattern (easing the cognitive load) but also it better communicates the intent: the function discards the result of the ioctl call and always returns null. It just happens that result is always null at this point, but this is not relevant. Similarly, I added the explicit argument mutate_flag: true to the getter calls to emphasize that the call is expected to return data in-place. It happens that true is the default argument, but I still prefer to be explicit, for the sake of clarity of intentions. Note that the setter calls don't set that argument because there are no expectations around its value.

@BCSharp BCSharp merged commit 368df74 into IronLanguages:main Feb 10, 2025
8 checks passed
@BCSharp BCSharp deleted the termios_tcgetwinsize branch February 10, 2025 18:02
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

Successfully merging this pull request may close these issues.

2 participants