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

[enhancement] Make formatting work with latest clang #2174

Closed
Airblader opened this issue Jan 25, 2016 · 9 comments · Fixed by #2547
Closed

[enhancement] Make formatting work with latest clang #2174

Airblader opened this issue Jan 25, 2016 · 9 comments · Fixed by #2547

Comments

@Airblader
Copy link
Member

We currently require clang35 to be used for formatting the code. I wonder whether we can make a newer version behave the same (as is, the formatting changes).

@stapelberg
Copy link
Member

Or we could just not bother and have a flag day where we switch from one to the other :).

@Airblader
Copy link
Member Author

And change the formatting? SGTM. When? :)

@stapelberg
Copy link
Member

Once I see proof that the version of clang-format you have in mind is available on travis, Debian testing and Arch Linux :).

@Airblader
Copy link
Member Author

Arch won't be a problem. It seems like Travis isn't there yet, though.

stapelberg added a commit to stapelberg/i3 that referenced this issue Feb 5, 2016
We now build a docker base container based on debian sid (where the very
latest packages are available). That base container is updated once a
month, or whenever travis-build.Dockerfile or debian/control change, but
re-used for subsequent travis runs. While the initial build might take
up to 15 minutes, subsequent builds typically run in a minute or two.

All the different steps that we run on travis are now factored into
separate scripts in the travis/ directory.

Switching to docker should also help with issue i3#2174.
stapelberg added a commit that referenced this issue Feb 6, 2016
We now build a docker base container based on debian sid (where the very
latest packages are available). That base container is updated once a
month, or whenever travis-build.Dockerfile or debian/control change, but
re-used for subsequent travis runs. While the initial build might take
up to 15 minutes, subsequent builds typically run in a minute or two.

All the different steps that we run on travis are now factored into
separate scripts in the travis/ directory.

Switching to docker should also help with issue #2174.
@Airblader
Copy link
Member Author

@stapelberg How does that Travis commit help with clang? :)

@stapelberg
Copy link
Member

It uses debian sid for builds, which has clang 3.6.

@Airblader
Copy link
Member Author

Ah, I see.

@stapelberg
Copy link
Member

Filed https://llvm.org/bugs/show_bug.cgi?id=30353 for the unexpected line breaks found in #2335.

@stapelberg
Copy link
Member

clang-format-3.5 was now removed from Debian unstable. We’ll need to upgrade to clang-format ≥ 3.7 or work around this issue in our travis config.

@stapelberg stapelberg self-assigned this Nov 8, 2016
stapelberg added a commit to stapelberg/i3 that referenced this issue Nov 8, 2016
https://llvm.org/bugs/show_bug.cgi?id=30353 was filed for the unintended
line break between in e.g. “TAILQ_ENTRY(foo)\nbar;”.

Until that’s fixed or a workaround is known, we’ll live with line
breaks. To make it a bit easier for readers to see what’s going on, I
added extra line breaks around each such struct member/variable
definition, so that they at least visually are a single unit.

fixes i3#2174
stapelberg added a commit that referenced this issue Nov 8, 2016
https://llvm.org/bugs/show_bug.cgi?id=30353 was filed for the unintended
line break between in e.g. “TAILQ_ENTRY(foo)\nbar;”.

Until that’s fixed or a workaround is known, we’ll live with line
breaks. To make it a bit easier for readers to see what’s going on, I
added extra line breaks around each such struct member/variable
definition, so that they at least visually are a single unit.

fixes #2174
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants