If user has specified config file, don't warn about missing cachedir. As an app that wants to load only custom fonts, I can add a minimal config file like this: <?xml version="1.0"?> <!DOCTYPE fontconfig SYSTEM "fonts.dtd"> <fontconfig> <dir>../fonts</dir> <cachedir prefix="xdg">fontconfig</cachedir> </fontconfig> but really, I don't care about cachedir. Since caching is safe (it takes care of stale caches etc), user should be able to leave it out. Warning about default config not having cachedir is different. Even then I'm not sure it's a good idea.
think of warning related to things in the configuration file, it can be dropped perhaps? because we can read the proper place configured at the build time if it's not specified, even on Win32, it should works the desired way after f6e6a8a. I'm not sure what's the purpose of the warning/error for that though, if it is to detect the installation error, is it a good idea to add an API like FcInitLoadConfigFromMemory() or so and allow NULL for the configuration? dunno if there are any cases to read it from memory really. FcInitWithMinimalConfig() or so may be more useful perhaps. Any thought?
(In reply to Akira TAGOH from comment #1) > > I'm not sure what's the purpose of the warning/error for that though, if it > is to detect the installation error, It might be ok for the system-wide config file. I suppose the aim is to make sure systems set global cachedir, as to prevent every user to have their own cache of all the system fonts. But we should definitely skip the warning for custom config files. > is it a good idea to add an API like FcInitLoadConfigFromMemory() That API is certainly desirable. > or so and allow NULL for the configuration? What's the NULL has to do here? Oh you mean creating a FcConfig with no configuration whatsoever? FcConfigCreate() does that. > dunno if there are any cases to read it from memory really. Reading from memory is useful. > FcInitWithMinimalConfig() or so may be more useful perhaps. > > Any thought? What would go into the minimal config? I discussed this in detail in my blog post. I don't think there's a clear line between minimal config and full config.
(In reply to Behdad Esfahbod from comment #2) > (In reply to Akira TAGOH from comment #1) > > > > I'm not sure what's the purpose of the warning/error for that though, if it > > is to detect the installation error, > > It might be ok for the system-wide config file. I suppose the aim is to > make sure systems set global cachedir, as to prevent every user to have > their own cache of all the system fonts. But we should definitely skip the > warning for custom config files. Right. but there are no way to know if it is custom or not. hence: > > is it a good idea to add an API like FcInitLoadConfigFromMemory() > > That API is certainly desirable. the above and the following suggestion. we can use it internally and move warnings out of it. > > FcInitWithMinimalConfig() or so may be more useful perhaps. > > > > Any thought? > > What would go into the minimal config? > > I discussed this in detail in my blog post. I don't think there's a clear > line between minimal config and full config. read it. well, I meant to make it convenient but may be better leave it to users then. so just reading the config from memory may be good.
(In reply to Akira TAGOH from comment #3) > (In reply to Behdad Esfahbod from comment #2) > > (In reply to Akira TAGOH from comment #1) > > > > > > I'm not sure what's the purpose of the warning/error for that though, if it > > > is to detect the installation error, > > > > It might be ok for the system-wide config file. I suppose the aim is to > > make sure systems set global cachedir, as to prevent every user to have > > their own cache of all the system fonts. But we should definitely skip the > > warning for custom config files. > > Right. but there are no way to know if it is custom or not. hence: It kinda is. If FONTCONFIG_FILE / FONTCONFIG_PATH are not set, and we were initializing default config, we should warn. > > > is it a good idea to add an API like FcInitLoadConfigFromMemory() > > > > That API is certainly desirable. > > the above and the following suggestion. we can use it internally and move > warnings out of it. > > > > FcInitWithMinimalConfig() or so may be more useful perhaps. > > > > > > Any thought? > > > > What would go into the minimal config? > > > > I discussed this in detail in my blog post. I don't think there's a clear > > line between minimal config and full config. > > read it. well, I meant to make it convenient but may be better leave it to > users then. so just reading the config from memory may be good. My current thinking is that for most use cases, reading full config (but not fonts) might be what user wants. The major exception would be font viewers, which probably can do without any config.
(In reply to Behdad Esfahbod from comment #4) > It kinda is. If FONTCONFIG_FILE / FONTCONFIG_PATH are not set, and we were > initializing default config, we should warn. "and" ? is it the only way to load the custom config right? even though one can load the default config that way as well, we have no idea to see if it's the default config or not. just to check these env vars: diff --git a/src/fcinit.c b/src/fcinit.c index 6134ed4..0f13ec3 100644 --- a/src/fcinit.c +++ b/src/fcinit.c @@ -91,12 +91,23 @@ FcInitLoadOwnConfig (FcConfig *config) { FcChar8 *prefix, *p; size_t plen; + FcBool have_own = FcFalse; + char *env_file, *env_path; - fprintf (stderr, - "Fontconfig warning: no <cachedir> elements found. Check configuration.\n"); - fprintf (stderr, - "Fontconfig warning: adding <cachedir>%s</cachedir>\n", - FC_CACHEDIR); + env_file = getenv ("FONTCONFIG_FILE"); + env_path = getenv ("FONTCONFIG_PATH"); + if ((env_file != NULL && env_file[0] != 0) || + (env_path != NULL && env_path[0] != 0)) + have_own = FcTrue; + + if (!have_own) + { + fprintf (stderr, + "Fontconfig warning: no <cachedir> elements found. Check configuration.\n"); + fprintf (stderr, + "Fontconfig warning: adding <cachedir>%s</cachedir>\n", + FC_CACHEDIR); + } prefix = FcConfigXdgCacheHome (); if (!prefix) goto bail; @@ -107,8 +118,9 @@ FcInitLoadOwnConfig (FcConfig *config) prefix = p; memcpy (&prefix[plen], FC_DIR_SEPARATOR_S "fontconfig", 11); prefix[plen + 11] = 0; - fprintf (stderr, - "Fontconfig warning: adding <cachedir prefix=\"xdg\">fontconfig</cachedir>\n"); + if (!have_own) + fprintf (stderr, + "Fontconfig warning: adding <cachedir prefix=\"xdg\">fontconfig</cachedir>\n"); if (!FcConfigAddCacheDir (config, (FcChar8 *) FC_CACHEDIR) || !FcConfigAddCacheDir (config, (FcChar8 *) prefix)) > My current thinking is that for most use cases, reading full config (but not > fonts) might be what user wants. The major exception would be font viewers, > which probably can do without any config. As you explained on your blog, it can be done without this change right? doing something with API sounds better than settings env vars and would be sane though.
(In reply to Akira TAGOH from comment #5) > (In reply to Behdad Esfahbod from comment #4) > > It kinda is. If FONTCONFIG_FILE / FONTCONFIG_PATH are not set, and we were > > initializing default config, we should warn. > > "and" ? is it the only way to load the custom config right? even though one > can load the default config that way as well, we have no idea to see if it's > the default config or not. just to check these env vars: > > diff --git a/src/fcinit.c b/src/fcinit.c > index 6134ed4..0f13ec3 100644 > --- a/src/fcinit.c > +++ b/src/fcinit.c > @@ -91,12 +91,23 @@ FcInitLoadOwnConfig (FcConfig *config) > { > FcChar8 *prefix, *p; > size_t plen; > + FcBool have_own = FcFalse; > + char *env_file, *env_path; > > - fprintf (stderr, > - "Fontconfig warning: no <cachedir> elements found. Check > configuration.\n"); > - fprintf (stderr, > - "Fontconfig warning: adding <cachedir>%s</cachedir>\n", > - FC_CACHEDIR); > + env_file = getenv ("FONTCONFIG_FILE"); > + env_path = getenv ("FONTCONFIG_PATH"); > + if ((env_file != NULL && env_file[0] != 0) || > + (env_path != NULL && env_path[0] != 0)) > + have_own = FcTrue; > + > + if (!have_own) > + { > + fprintf (stderr, > + "Fontconfig warning: no <cachedir> elements found. Check > configuration.\n"); > + fprintf (stderr, > + "Fontconfig warning: adding <cachedir>%s</cachedir>\n", > + FC_CACHEDIR); > + } > prefix = FcConfigXdgCacheHome (); > if (!prefix) > goto bail; > @@ -107,8 +118,9 @@ FcInitLoadOwnConfig (FcConfig *config) > prefix = p; > memcpy (&prefix[plen], FC_DIR_SEPARATOR_S "fontconfig", 11); > prefix[plen + 11] = 0; > - fprintf (stderr, > - "Fontconfig warning: adding <cachedir > prefix=\"xdg\">fontconfig</cachedir>\n"); > + if (!have_own) > + fprintf (stderr, > + "Fontconfig warning: adding <cachedir > prefix=\"xdg\">fontconfig</cachedir>\n"); > > if (!FcConfigAddCacheDir (config, (FcChar8 *) FC_CACHEDIR) || > !FcConfigAddCacheDir (config, (FcChar8 *) prefix)) > > > My current thinking is that for most use cases, reading full config (but not > > fonts) might be what user wants. The major exception would be font viewers, > > which probably can do without any config. > > As you explained on your blog, it can be done without this change right? I mean, people can include a cachedir. It just made more sense to not warn. > doing something with API sounds better than settings env vars and would be > sane though. Both are useful. Thanks.
Patch looks good BTW.
committed the change into git. thanks.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.