Skip to content

clang-format generated C bindings #736

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

Closed
mahesh-hegde opened this issue Oct 2, 2022 · 4 comments · Fixed by dart-archive/jnigen#87
Closed

clang-format generated C bindings #736

mahesh-hegde opened this issue Oct 2, 2022 · 4 comments · Fixed by dart-archive/jnigen#87
Labels
type-question A question about expected behavior or functionality

Comments

@mahesh-hegde
Copy link
Contributor

It's a good idea if generated bindings are formatted consistently.

In theory we can run clang-format on generated .c file once in the generator.

In practice user may open the file in an editor with auto format (Eg VSCode with C++ plugin). Then it will reformat the file. (Yes user should not edit the generated file. But one might open the file and press Ctrl+S out of habit, which will reformat.)

Is it a good idea to copy the .clang-format file from jni package into generated c bindings folder, and format according to that?

@mahesh-hegde mahesh-hegde added the type-question A question about expected behavior or functionality label Oct 2, 2022
@mahesh-hegde
Copy link
Contributor Author

mahesh-hegde commented Oct 3, 2022

@dcharkes

Approach 1: Do not format generated bindings

++ No requirement of clang-format tool
-- No consistent formatting

Approach 2: Format using default config

++ No config file required
-- Mismatch with C/C++ style used by dart SDK and package:jni
-- Requires clang-format on user's end.

Approach 3: Copy .clang-format from jni into generated code directory.

++ Consistent with other packages
-- Requires clang-format on user's end.

@dcharkes
Copy link
Collaborator

dcharkes commented Oct 3, 2022

How about we try if we find clang-format and then use it, if we don't find it we output a warning but still continue.

Since we generate the code we could dictate the formatting. Do we need to copy the .clang-format file in, or can we pass clang-format a config file with an argument?

@mahesh-hegde
Copy link
Contributor Author

mahesh-hegde commented Oct 3, 2022

Do we need to copy the .clang-format file in

It's not strictly necessary. But imagine a situation

  • jnigen generated and formatted C binding with custom config

  • User opens C file in VS code

  • Doesn't change anything but press Ctrl+S out of habit

  • VS code reformats using default clang-format config.

I think it's better to save the custom config than passing it as one time through command line.

@dcharkes
Copy link
Collaborator

dcharkes commented Oct 3, 2022

Yeah that's true.

I wouldn't mind having a config file. I think it is good practise to specify the way code should be formatted, such file does exactly that.

If anyone later on doesn't like it, we can always add a config option to not generate the the formatting file.

dcharkes referenced this issue in dart-archive/jnigen Oct 7, 2022
* C bindings no longer use fully qualified names. Instead, it uses number-renamed simple names. The classes are sorted in the summarizer to ensure the order is deterministic. This enables shorter symbol names and less verbose bindings.

* C bindings are formatted using same style as Dart SDK, using clang-format. If clang-format is not available, jnigen will issue a warning.
Closes: #84

* C bindings return `JniResult`. The older `Jni.checkException` stopgap is removed. It reduces one line per function binding in dart. Lastly exceptions will work properly on android
Closes: #56
HosseinYousefi referenced this issue Nov 16, 2023
…port. (dart-archive/jnigen#87)

* C bindings no longer use fully qualified names. Instead, it uses number-renamed simple names. The classes are sorted in the summarizer to ensure the order is deterministic. This enables shorter symbol names and less verbose bindings.

* C bindings are formatted using same style as Dart SDK, using clang-format. If clang-format is not available, jnigen will issue a warning.
Closes: https://github.com/dart-lang/jnigen/issues/84

* C bindings return `JniResult`. The older `Jni.checkException` stopgap is removed. It reduces one line per function binding in dart. Lastly exceptions will work properly on android
Closes: https://github.com/dart-lang/jnigen/issues/56
@HosseinYousefi HosseinYousefi transferred this issue from dart-archive/jnigen Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-question A question about expected behavior or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants