Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review use of \xspace in macro definitions #1433

Closed
jensmaurer opened this issue Feb 5, 2017 · 5 comments
Closed

Review use of \xspace in macro definitions #1433

jensmaurer opened this issue Feb 5, 2017 · 5 comments
Assignees
Milestone

Comments

@jensmaurer
Copy link
Member

See http://tex.stackexchange.com/questions/86565/drawbacks-of-xspace with the first answer from the package author of \xspace. Quote:

So, if you find it useful, fine, it's there. But personally I wouldn't recommend it.

\xspace in \grammarterm caused spacing issues while addressing #1268.

@godbyk
Copy link
Contributor

godbyk commented Feb 5, 2017

Since \grammarterm takes a mandatory argument, there's no need for \xspace there. \xspace is used primarily in macros that take no argument and expand to text. For example, consider the following:

\newcommand{\myname}{Kevin}% no \xspace
My name is \myname. \myname is my name.
% \myname swallows and following white space
% such that the following word is adjoined to the
% expanded 'Kevin' text producing,
% 'Kevinis my name.'
%
% You work around this by using doing one of the
% following:
\myname\ is my name.
\myname{} is my name.

% ... or use \xspace:
\newcommand{\mynamex}{Kevin\xspace}
My name is \mynamex. \mynamex is my name.

It looks like folks have been working around this issue by wrapping the s in braces to prevent \xspace from inserting space. Compare:

\grammarterm{blah}s
\grammarterm{blah}{s}

In this case, it'd be better to just remove the \xspace macro from \grammarterm.

@godbyk
Copy link
Contributor

godbyk commented Feb 5, 2017

The following uses of \xspace look questionable to me:

\newcommand{\techterm}[1]{\textit{#1}\xspace}
\newcommand{\defnx}[2]{\indexdefn{#2}\textit{#1}\xspace}
\newcommand{\term}[1]{\textit{#1}\xspace}
\newcommand{\grammarterm}[1]{\textit{#1}\xspace}
\newcommand{\Fundescx}[1]{\textit{#1}\xspace}
\newcommand{\NTS}[1]{\textsc{#1}\xspace}
\newcommand{\Range}[4]{\tcode{#1#3,\penalty2000{} #4#2}\xspace}
\newcommand{\diffdef}[1]{\hfill\break\textbf{#1:}\xspace}
\newcommand{\stage}[1]{\item{\textbf{Stage #1:}}\xspace}
\newcommand{\doccite}[1]{\textit{#1}\xspace}
\renewcommand{\emph}[1]{\textit{#1}\xspace}
\newcommand{\numconst}[1]{\textsl{#1}\xspace}
\newcommand{\logop}[1]{{\footnotesize #1}\xspace}
\newcommand{\terminal}[1]{{\BnfTermshape ##1}\xspace}

These uses are probably fine:

\newcommand{\Cpp}{\texorpdfstring{C\kern-0.05em\protect\raisebox{.35ex}{\textsmaller[2]{+\kern-0.05em+}}}{C++}\xspace}
\newcommand{\CppIII}{\Cpp 2003\xspace}
\newcommand{\CppXI}{\Cpp 2011\xspace}
\newcommand{\CppXIV}{\Cpp 2014\xspace}
\newcommand{\opt}{{\ensuremath{_\mathit{opt}}}\xspace}
\newcommand{\xref}{\textsc{See also:}\xspace}

Slightly iffy depending on the expected behavior of the environments:

\newenvironment{note}[1][Note]{\noteintro{#1}}{\noteoutro{note}\xspace}
\newenvironment{example}[1][Example]{\noteintro{#1}}{\noteoutro{example}\xspace}

@zygoloid
Copy link
Member

zygoloid commented Feb 6, 2017

I'd like to move away from xspace entirely. Over-reliance on the \xspace in \opt has caused formatting issues in grammar snippets in the past. I think we just need to get into the habit of writing it \opt{} -- or even \opt{optional thing}.

@godbyk
Copy link
Contributor

godbyk commented Feb 6, 2017

@zygoloid That's probably the better option. I would document the meaning of the empty braces, though, so that future authors don't remove them out of ignorance.

In my lists above, the 'questionable' uses of \xspace should be safe to remove as the \xspace won't have much appreciable impact (aside from kerning changes). The 'probably fine' uses require each use to be evaluated individually (some will need to be rewritten with {} or \space). I don't recall if \xspace can see past the end of an environment well enough to function consistently or not, so I would have to check and see what impact removing \xspace has there.

@jensmaurer
Copy link
Member Author

I like \opt{thing}, it's much more content-focused than \opt{}.

jensmaurer added a commit to jensmaurer/draft that referenced this issue Feb 7, 2017
@jensmaurer jensmaurer self-assigned this Feb 7, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Feb 8, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Feb 9, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Feb 15, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 15, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 18, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 18, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 19, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 24, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 25, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 26, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 27, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Feb 27, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 27, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 27, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Feb 28, 2017
@jensmaurer jensmaurer added this to the C++20 milestone Mar 2, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Mar 2, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 2, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 3, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 3, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 3, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Mar 24, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 25, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 28, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 28, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Mar 31, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Apr 6, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Apr 7, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Apr 8, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Apr 8, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Apr 15, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue May 1, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue May 31, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Jun 17, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Jun 20, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Jul 1, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Jul 17, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Jul 17, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Jul 24, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Jul 24, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Jul 25, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Jul 26, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Jul 27, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Jul 28, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Jul 31, 2017
jensmaurer added a commit to jensmaurer/draft that referenced this issue Aug 1, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Aug 11, 2017
Eelis pushed a commit to Eelis/draft that referenced this issue Aug 12, 2017
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

No branches or pull requests

3 participants