Skip to content

Conversation

@ttytm
Copy link
Contributor

@ttytm ttytm commented Oct 8, 2023

Adds changes mentioned in: #242

I think the settings resemble the current style fairly well.

A style I couldn't find a matching setting for is to indent everything in pre-processor directives. Adding IndentPPDirectives: BeforeHash to .clang-format would come closest, but variable declarations still wouldn't be indent, resulting in:

  • Uniform indent without decls:

    #if defined(__clang__) || defined(__GNUC__)
    	#define WEBUI_DISABLE_OPTIMIZATION_START _Pragma("GCC optimize (\"O0\")")
    	#define WEBUI_DISABLE_OPTIMIZATION_END
    #elif defined(_MSC_VER)
    	#define WEBUI_DISABLE_OPTIMIZATION_START __pragma(optimize("", off))
    	#define WEBUI_DISABLE_OPTIMIZATION_END   __pragma(optimize("", on))
    #else
    	#define WEBUI_DISABLE_OPTIMIZATION_START
    	#define WEBUI_DISABLE_OPTIMIZATION_END
    #endif
  • Not uniform indent for decls:

    #ifdef _WIN32
    static const char* webui_sep = "\\";
    static DWORD WINAPI _webui_run_browser_task(LPVOID _arg);
    static int _webui_system_win32(_webui_window_t* win, char* cmd, bool show);
    static int _webui_system_win32_out(const char* cmd, char** output, bool show);
    static bool _webui_socket_test_listen_win32(size_t port_num);
    static bool _webui_get_windows_reg_value(HKEY key, LPCWSTR reg, LPCWSTR value_name, char value[WEBUI_MAX_PATH]);
    
    	#define WEBUI_THREAD_SERVER_START DWORD WINAPI _webui_server_start(LPVOID arg)
    	#define WEBUI_THREAD_RECEIVE      DWORD WINAPI _webui_receive_thread(LPVOID _arg)
    	#define WEBUI_THREAD_RETURN       return 0;
    #else
  • So I left the setting out to use the default / more commonly used style of not indenting pre-processor directives. The used sections for the directives are usually highlighted by the editor if an lsp is running:

    Screenshot_20231008_032820

If any changes would further improve things for you let me know.

Example runs:

@ttytm ttytm force-pushed the format branch 2 times, most recently from f497d8d to 79783e5 Compare October 8, 2023 01:55
@ttytm
Copy link
Contributor Author

ttytm commented Oct 8, 2023

Microsoft is a space nazi when there is no config that says otherwise. Here is where you can configure tab width on github to use what you like best for viewing code in the browser: https://github.com/settings/appearance

Screenshot_20231008_035944

@ttytm ttytm requested a review from hassandraga October 9, 2023 09:26
@ttytm ttytm merged commit 5e7c407 into webui-dev:main Oct 9, 2023
@ttytm ttytm deleted the format branch October 9, 2023 15:34
@ttytm
Copy link
Contributor Author

ttytm commented Oct 10, 2023

Currently the lint assumed that clangd installed used locally, then all common editors could take care of formatting via format on save or running a format command.

I didn't integrated formatting and writing to the repository if formatting is off. We can do it if that's preferred.

@hassandraga
Copy link
Member

Formating on CI is better as we can not guarantee that all contributors have Clang installed.

@ttytm
Copy link
Contributor Author

ttytm commented Oct 10, 2023

Okay, I see that automated writing of formatting changes can make sense in our case. I'll look into it.

Though it is a valid requirement for contribution to meet a projects code style. And it's not guaranteed for most formatters unless a formatter is built into the language itself. Alternatively, a contributor could format manually to satisfy linting for now before a PR is merged if the linter is not happy.

@hassandraga
Copy link
Member

I mean, whatever is easier and efficient we should do it. We can format PR manually.

@hassandraga
Copy link
Member

I will use Clang to format code for my next PRs.

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.

2 participants