Skip to content

Refactor/plugin header split#1506

Open
pgrandin wants to merge 12 commits into
trunkfrom
refactor/plugin-header-split
Open

Refactor/plugin header split#1506
pgrandin wants to merge 12 commits into
trunkfrom
refactor/plugin-header-split

Conversation

@pgrandin

Copy link
Copy Markdown
Contributor

Addresses #1439
Our current code was maybe slightly abusing the list macros here.

Comment thread navit/plugin_impl.h

/**
* @file plugin_impl.h
* @brief Plugin implementation macros — included only by plugin.c.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It its only included inside plugin.c why do we need a header for it?

@pgrandin pgrandin Mar 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It felt cleaner that way, keeping the .c file focused on the actual logic instead of dealing with macros. The impl header also needs to #undef and redefine several macros before re-expanding plugin_def.h a second time so that's ~60 lines of macro machinery that could clutter plugin.c if inlined.

What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think, having it is fine and for my taste plugin.c still contains too many macros/defines (which is no way speaking to this PR but rather to the structure that is in place).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I did notice there is still some kind of problem around plugin_def.h. If that one receives an include guard, linking fails. I have not been able to look into this further

@pgrandin pgrandin self-assigned this Mar 19, 2026
@christf

christf commented Mar 23, 2026

Copy link
Copy Markdown
Collaborator

wow, thank you for this! :-)

Comment thread navit/plugin_impl.h
#define NAVIT_PLUGIN_IMPL_H

#include "plugin.h"
#include <glib.h>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think including "glib.h" is better here, if we want to continue supporting platforms without native glib.

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