After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 640449 - Include config.h only when needed
Include config.h only when needed
Status: RESOLVED OBSOLETE
Product: gedit
Classification: Applications
Component: general
git master
Other Linux
: Normal enhancement
: ---
Assigned To: Gedit maintainers
Gedit maintainers
Depends on:
Blocks:
 
 
Reported: 2011-01-24 17:52 UTC by Sébastien Wilmet
Modified: 2020-11-24 09:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (3.29 KB, patch)
2011-01-24 17:52 UTC, Sébastien Wilmet
none Details | Review

Description Sébastien Wilmet 2011-01-24 17:52:28 UTC
Created attachment 179200 [details] [review]
Patch

I saw that config.h is included in a lot of files, but sometimes it is not needed.
To be sure that it's not needed, here is what I did:

grep -e "^#undef" config.h.in | cut -d " " -f 2 > config.list
./verif_config_removed.sh config.list gedit/file.c

With verif_config_removed.sh:
#!/usr/bin/env bash
cat $1 | xargs -i grep -n --color {} $2

So I took config.h.in to generate a list of symbols and then grep for each symbol in a particular file.


The patch cleans only a few files, I'll do the others as I advance with the libgedit.
Comment 1 Ignacio Casal Quinteiro (nacho) 2011-10-08 22:24:31 UTC
mmmm, feel free to provide an updated patch, anyway it is not much of a problem.
Comment 2 Tobias Mueller 2012-02-01 15:22:05 UTC
So is this bug a WONTFIX and the patch rejected?
Comment 3 Paolo Borelli 2012-03-04 16:13:32 UTC
yes, we usually keep config.h in all the files for consistency
Comment 4 Sébastien Wilmet 2014-09-02 13:29:32 UTC
I reopen this bug, I think it's quite useful to know that a file doesn't depend on config.h to make this file reusable (for the libgedit project for example).
Comment 5 Garrett Regier 2014-10-11 10:29:27 UTC
Including config.h is actually required for any file that uses any of glib's many logging functions due to G_LOG_DOMAIN, including:

  - g_return_*if_*()
  - g_warn_*if_*()
  - g_debug()
  - g_info()
  - g_warning()
  - g_critical()
  - g_message()
  - g_error()


Which means pretty much every file will need this and makes it simply easier to avoid mistakes which it isn't included.
Comment 6 Sébastien Wilmet 2014-10-11 11:42:51 UTC
Ok, now I better understand why it's almost always included.
Comment 7 Sébastien Wilmet 2014-12-20 15:48:12 UTC
I come back again on this bug. G_LOG_DOMAIN isn't defined in gedit's config.h. For an application it is generally not defined, as the doc of G_LOG_DOMAIN explains. For a library (at least GTK+ and GtkSourceView), it is neither defined in config.h, since G_LOG_DOMAIN is passed in the CPPFLAGS.

So I think it is safe to remove the useless config.h includes.
Comment 8 Swapnesh Kumar Sahoo 2017-03-05 19:32:14 UTC
I'd like to work on this bug. I am a beginner.
Comment 9 Sébastien Wilmet 2020-11-24 09:57:27 UTC
Mass-closing of all gedit bugzilla tickets.

Special "code" to find again all those gedit bugzilla tickets that were open before the mass-closing:

2bfe1b0590a78457e1f1a6a90fb975f5878cb60064ccfe1d7db76ca0da52f0f3

By searching the above sha256sum in bugzilla, the gedit contributors can find again the tickets. We may be interested to do so when we work on a specific area of the code, to at least know the known problems and possible enhancements.

We do this mass-closing because bugzilla.gnome.org is being replaced by gitlab.gnome.org.