<div dir="ltr">Hi Kristian,<div><br></div><div class="gmail_extra"><div class="gmail_quote">On Tue, May 14, 2013 at 1:57 PM, Kristian Høgsberg <span dir="ltr"><<a href="mailto:hoegsberg@gmail.com" target="_blank">hoegsberg@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On Tue, May 14, 2013 at 09:55:19AM -0700, Othman, Ossama wrote:<br>
> I've attached the patch since the patch I sent through git send-mail was<br>
> sent through an e-mail account that isn't subscribed to this list (waiting<br>
> for moderator approval), and to make sure the spacing isn't munged again.<br>
> This patch was generated against weston master as of this morning.<br>
<br>
</div>Cool, looks good now, committed. I did catch a corner case in the<br>
spec that we could handle better. As far as I can see,<br>
XDG_CONFIG_HOME, if set, overrides ~/.config. That means if it's set<br>
and we don't find a config file there, we shouldn't fall back and try<br>
~/.config/weston.ini.</blockquote><div><br></div><div style>Good catch!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Similarly, only if XDG_CONFIG_DIRS is not set<br>
do we fall back to /etc/xdg, but we do handle correctly with your<br>
patch. Also the spec says "not set or empty", so we should be<br>
checking !config_dirs || *config_dirs == '\0'.<br>
<br>
Finally, I didn't see a reason for adding weston/ to the path when<br>
iterating the XDG_CONFIG_DIRS? I guess many applications put their<br>
config file in a subdirectory of the config dir, but at least for<br>
~/.config/weston.ini we don't do that.<br></blockquote><div><br></div><div style>The part of the XDG base spec that discusses XDG_CONFIG_DIRS wasn't very clear to me. I was thrown by the mention of "subdirs" on the web page at <a href="http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html#referencing">http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html#referencing</a>. Unfortunately "subdirs" doesn't appear to be defined. It states:</div>
<div style><p> Specifications may reference this specification by specifying the
location of a configuration file as
<code class="">$XDG_CONFIG_DIRS</code>/subdir/filename. This implies that:
</p><div class=""><ul type="disc"><li><p>
Default configuration files should be installed to <code class="">$sysconfdir</code>/xdg/subdir/filename
with <code class="">$sysconfdir</code> defaulting to /etc.
</p></li><li><p>
A user specific version of the configuration file may be created in
<code class="">$XDG_CONFIG_HOME</code>/subdir/filename, taking into
account the default value for <code class="">$XDG_CONFIG_HOME</code> if
<code class="">$XDG_CONFIG_HOME</code> is not set.
</p></li><li><p>
Lookups of the configuration file should search for ./subdir/filename relative to
all base directories indicated by <code class="">$XDG_CONFIG_HOME</code> and
<code class="">$XDG_CONFIG_DIRS</code> . If an environment
variable is either not set or empty, its default value as defined by this specification
should be used instead. </p></li></ul></div></div><div style>I interpreted "subdir" as "weston" in our case. My Linux box (Ubuntu 12.10) also puts config files under various subdirectories in /etc/xdg; although there are some config file placed directly in /etc/xdg as well. I don't know which is right so I'm happy with whatever you decide. :)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I committed a small patch to fix up these issues - let me know if you<br>
see a problem.<br></blockquote><div><br></div><div style>Will do!</div><div style><br></div><div style>Thanks Kristian!</div><div style>-Ossama</div><div style><br></div></div></div></div>