Skip to content

Conversation

@cpplearner
Copy link
Contributor

  1. Change "noskipws is zero" to "noskipws is false". The latter is more conventional since noskipws is a bool.
  2. Rename the variable ctype to ct. This prevents it from shadowing the class template std::ctype.
  3. Remove redundant comparison between the result of ct.is(…) (which has type bool) and 0.
  4. Add a semicolon to make the sentence structure more clear.

@tkoeppe tkoeppe requested a review from jwakely November 1, 2025 18:23
@eisenwave eisenwave added the P2-Bug Presentational errors and omissions label Nov 6, 2025
is
\tcode{true},
\tcode{\exposid{ok_} != false}
\tcode{\exposid{ok_} != false};
Copy link
Member

@eisenwave eisenwave Nov 6, 2025

Choose a reason for hiding this comment

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

Suggested change
\tcode{\exposid{ok_} != false};
\tcode{\exposid{ok_}} is \tcode{false};

The == false below should also be changed for local consistency; then we have local consistency about using "is true"/"is false" in this \itemdescr. Not fixing this would be silly if we're already touching this line.

Copy link
Contributor

Choose a reason for hiding this comment

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

The original wording is awfully terse. Can we spell this out a bit more, like:

"""
If, after any preparation is completed, is.good() is true, then ok_ is true, otherwise ok_ is false.
"""

Or could we deunpack this even further and say something like "the value of ok_ is is.good()."?

@jwakely?

Copy link
Member

Choose a reason for hiding this comment

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

Well we definitely can't change "ok_ != false" to "ok_ is false"!

I agree with tkoeppe's suggestion, including adding "then" after the third comma.

We could also just say:

After any preparation is completed, ok_ is set to the value of is.good().

Just saying ok_ = is.good() seems much simpler than describing the following logic, which would never pass code review:

if (is.good() == true)
  ok_ = !false;
else
  ok_ = false;

Copy link
Member

Choose a reason for hiding this comment

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

Anything would be better than what's in the draft today ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, this sentence should probably be part of the effects instead of remarks?

whitespace or not.

\pnum
To decide if the character \tcode{c} is a whitespace character,
Copy link
Contributor

Choose a reason for hiding this comment

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

While we are here, we can also drop the quote of the constructor signature in the previous paragraph.

Like...it's a Remarks for this constructor. Do we really need to say yet again that we are talking about this constructor?

Comment on lines 4592 to 4604
This constructor
uses the currently imbued locale in \tcode{is},
to determine whether the next input character is
whitespace or not.

\pnum
To decide if the character \tcode{c} is a whitespace character,
the constructor performs as if it executes the following code fragment:
\begin{codeblock}
const ctype<charT>& ctype = use_facet<ctype<charT>>(is.getloc());
if (ctype.is(ctype.space, c) != 0)
const ctype<charT>& ct = use_facet<ctype<charT>>(is.getloc());
if (ct.is(ct.space, c))
// \tcode{c} is a whitespace character.
\end{codeblock}
Copy link
Member

Choose a reason for hiding this comment

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

I think these are good improvements so far.

But these two paragraphs are saying the same thing, so I think it would be even more clear (and shorter) if we combined them:

Suggested change
The currently imbued locale in \tcode{is} is used
to determine whether or not the next input character is whitespace.
The character \tcode{c} is a whitespace character if
\tcode{ct.is(ct.space, c)} is \tcode{true},
where \tcode{ct} is \tcode{use_facet<ctype<charT>>(is.getloc())}.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @cpplearner and @jwakely for working on this -- this wording can definitely do with improvements, this is great!

\tcode{setstate(failbit | eofbit)}
(which may throw
\tcode{ios_base::failure}).
After any preparation is completed, \exposid{ok_} is set to the value of \tcode{is.good()}.
Copy link
Member

Choose a reason for hiding this comment

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

As this describes the final step, I think it would make more sense to be right at the end, after the "During preparation" sentence (maybe as a separate paragraph).

That way the structure would be:

  • The function extracts and discards each whitespace character.
  • If extracting a character returns eof, set failbit|eofbit.
  • Whitespace is determined using ctype.
  • The constructor may set failbit and throw. (for ... reasons? I guess the footnote covers that)
  • After any preparation is completed, set ok_.

What do you think?

Copy link
Contributor Author

@cpplearner cpplearner Nov 19, 2025

Choose a reason for hiding this comment

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

IMO this makes sense if we drop this \remarks macro.

\remarks

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, agreed.

Clarify whitespace character determination in constructor
Copy link
Member

@jwakely jwakely left a comment

Choose a reason for hiding this comment

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

I think this is a big improvement, thanks

// \tcode{c} is a whitespace character.
\end{codeblock}
The currently imbued locale in \tcode{is} is used
to determine whether or not the next input character is whitespace.
Copy link
Contributor

Choose a reason for hiding this comment

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

"Whether" already means "whether or not", so can we drop the "or not"?

On the other hand, "if" does not already mean "only if", so can we say "if and only if" below?

And maybe we can also change the full stop after "is whitespace" to a colon to signal that what follows is the explanation of what whitespace is?

\end{footnote}

\pnum
After any preparation is completed, \exposid{ok_} is set to the value of \tcode{is.good()}.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2-Bug Presentational errors and omissions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants