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

FMT_USE_FCNTL can be predefined #2572

Closed
wants to merge 1 commit into from
Closed

FMT_USE_FCNTL can be predefined #2572

wants to merge 1 commit into from

Conversation

timkalu
Copy link
Contributor

@timkalu timkalu commented Oct 28, 2021

'os.h' accepts a predefined FMT_USE_FCNTL override so using FMT with e.g. the NXP toolchain for ARM (e.g. for FreeRTOS) does not have a fcntl() call, but the detection routine does not detect this correctly.

'os.h'  accepts a predefined FMT_USE_FCNTL override so using FMT with e.g. the NXP toolchain for ARM (e.g. for FreeRTOS) does not have a fcntl() call, but the detection routine does not detect this correctly.
@timkalu
Copy link
Contributor Author

timkalu commented Oct 28, 2021

See issue #2570

# if (FMT_HAS_INCLUDE(<fcntl.h>) || defined(__APPLE__) || \
defined(__linux__)) && \
(!defined(WINAPI_FAMILY) || (WINAPI_FAMILY == WINAPI_FAMILY_DESKTOP_APP))
# i nclude <fcntl.h> // for O_RDONLY
Copy link
Contributor

Choose a reason for hiding this comment

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

Unexpected space in "include".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for that, I'll be more careful next time.

@timkalu
Copy link
Contributor Author

timkalu commented Oct 28, 2021

oh, sorry, I'll do that again.

@timkalu timkalu closed this Oct 28, 2021
@alexezeder
Copy link
Contributor

alexezeder commented Oct 28, 2021

Closing this PR and opening the new one was unnecessary. You could just change the line with this i nclude locally, commit it to the same branch (was patch-1 in your case), then push this commit to your fork. This is how you could update this PR, so the review becomes outdated, and you can resolve it. 😉
But now, when everything is deleted, this instruction would only work for the new PR, if there are any problems.

@timkalu
Copy link
Contributor Author

timkalu commented Oct 28, 2021

This was my very first commit/pull request ever, so I am glad I managed it at all 😸. I'll do better next time.

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.

3 participants