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.

Sign up for GitHub

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

Jump to bottom

[cmath.syn] Align function declarations #6543

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[cmath.syn] Align function declarations #6543

wants to merge 1 commit into from

Conversation

Eisenwave
Copy link
Contributor

@Eisenwave Eisenwave commented Aug 30, 2023 •
edited

Based on #6542.

If you look at the C++17 synopsis, you will notice that the mathematical special functions used to be beautifully aligned:

  double       ellint_1(double k, double phi);
  float        ellint_1f(float k, float phi);
  long double  ellint_1l(long double k, long double phi);

Unfortunately, the current working draft uses a placeholder floating-point-type which breaks that alignment:

  floating-point-type comp_ellint_1(floating-point-type k);
  float        comp_ellint_1f(float k);
  long double  comp_ellint_1l(long double k);

Also, the mathematical common functions have not yet been aligned, which makes them annoying to read in my opinion:

  constexpr floating-point-type cos(floating-point-type x);
  constexpr float cosf(float x);
  constexpr long double cosl(long double x);

This edit aligns all functions in the synopsis:

  constexpr floating-point-type cos(floating-point-type x);
  constexpr float               cosf(float x);
  constexpr long double         cosl(long double x);

@Eisenwave Eisenwave changed the title Align cmath [cmath.syn] Align function declarations Aug 30, 2023
@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 31, 2023

Try \placeholdernc instead of \placeholder?

@Eisenwave
Copy link
Contributor Author

Try \placeholdernc instead of \placeholder?

image

That seems to have done the trick. (above is placeholdernc, below is placeholder)

@Eisenwave
Copy link
Contributor Author

image
All nicely aligned now.

jensmaurer
jensmaurer approved these changes Sep 2, 2023
View reviewed changes
Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Squash when merging.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 10, 2023

Could you please rebase this?

@tkoeppe tkoeppe added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Nov 10, 2023
@tkoeppe
Copy link
Contributor

tkoeppe commented Dec 18, 2023

@Eisenwave Ping.

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

@jensmaurer jensmaurer jensmaurer approved these changes

Assignees

@tkoeppe tkoeppe

Labels
needs rebase The pull request needs a git rebase to resolve merge conflicts.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Eisenwave @tkoeppe @jensmaurer