You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rodent of Unusual Size <Ke...@Golux.Com> on 1998/09/25 21:28:02 UTC
[PATCH] Add '+' and '-' to IndexOptions keywords
Since I've been futzing around in mod_autoindex recently,
I thought I'd submit this thing I've had banging around
for a while (updated to 1.3.3-dev).
The attached patch enhances IndexOptions to accept
incremental keywords, like
IndexOptions -IconsAreLinks +SuppressLastModified
in a manner similar to the Options core directive. As
a side-effect, multiple IndexOptions directives are
combined, rather than each overriding the last, so the
above is equivalent to
IndexOptions -IconsAreLinks
IndexOptions +SuppressLastModified
One other semantic diference: if a keyword appears
without a '+' or '-' prefix, it totally clears any
existing incremental changes and sets the options to
that keyword. Subsequent keywords with prefixes
go into the add/remove buckets until the next unprefixed
keyword appears. That is, these are equivalent:
IndexOptions +IconsAreLinks +SuppressLastModified FancyIndexing
and
IndexOptions FancyIndexing
or
IndexOptions +IconsAreLinks FancyIndexing -SuppressLastModified
and
IndexOptions FancyIndexing -SuppressLastModified
Comments? This is a feature, so R-T-C applies, yes?
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Index: modules/standard/mod_autoindex.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_autoindex.c,v
retrieving revision 1.93
diff -u -r1.93 mod_autoindex.c
--- mod_autoindex.c 1998/09/24 15:53:40 1.93
+++ mod_autoindex.c 1998/09/25 19:16:18
@@ -131,6 +131,8 @@
char *default_icon;
int opts;
+ int incremented_opts;
+ int decremented_opts;
int name_width;
int name_adjust;
int icon_width;
@@ -294,6 +296,10 @@
cfg = (autoindex_config_rec *) d;
curopts = cfg->opts;
+ if (curopts & NO_OPTIONS) {
+ return "FancyIndexing directive conflicts with existing "
+ "IndexOptions None";
+ }
newopts = (arg ? (curopts | FANCY_INDEXING) : (curopts & ~FANCY_INDEXING));
cfg->opts = newopts;
return NULL;
@@ -302,51 +308,97 @@
static const char *add_opts(cmd_parms *cmd, void *d, const char *optstr)
{
char *w;
- int opts = 0;
+ int opts;
+ int opts_add;
+ int opts_remove;
+ char action;
autoindex_config_rec *d_cfg = (autoindex_config_rec *) d;
+ opts = d_cfg->opts;
+ opts_add = d_cfg->incremented_opts;
+ opts_remove = d_cfg->decremented_opts;
while (optstr[0]) {
+ int option = 0;
+
w = ap_getword_conf(cmd->pool, &optstr);
+ if ((*w == '+') || (*w == '-')) {
+ action = *(w++);
+ }
+ else {
+ action = '\0';
+ }
if (!strcasecmp(w, "FancyIndexing")) {
- opts |= FANCY_INDEXING;
+ option = FANCY_INDEXING;
}
else if (!strcasecmp(w, "IconsAreLinks")) {
- opts |= ICONS_ARE_LINKS;
+ option = ICONS_ARE_LINKS;
}
else if (!strcasecmp(w, "ScanHTMLTitles")) {
- opts |= SCAN_HTML_TITLES;
+ option = SCAN_HTML_TITLES;
}
else if (!strcasecmp(w, "SuppressLastModified")) {
- opts |= SUPPRESS_LAST_MOD;
+ option = SUPPRESS_LAST_MOD;
}
else if (!strcasecmp(w, "SuppressSize")) {
- opts |= SUPPRESS_SIZE;
+ option = SUPPRESS_SIZE;
}
else if (!strcasecmp(w, "SuppressDescription")) {
- opts |= SUPPRESS_DESC;
+ option = SUPPRESS_DESC;
}
else if (!strcasecmp(w, "SuppressHTMLPreamble")) {
- opts |= SUPPRESS_PREAMBLE;
+ option = SUPPRESS_PREAMBLE;
}
else if (!strcasecmp(w, "SuppressColumnSorting")) {
- opts |= SUPPRESS_COLSORT;
+ option = SUPPRESS_COLSORT;
}
else if (!strcasecmp(w, "None")) {
+ if (action != '\0') {
+ return "Cannot combine '+' or '-' with 'None' keyword";
+ }
opts = NO_OPTIONS;
+ opts_add = 0;
+ opts_remove = 0;
}
else if (!strcasecmp(w, "IconWidth")) {
- d_cfg->icon_width = DEFAULT_ICON_WIDTH;
+ if (action != '-') {
+ d_cfg->icon_width = DEFAULT_ICON_WIDTH;
+ }
+ else {
+ d_cfg->icon_width = 0;
+ }
}
else if (!strncasecmp(w, "IconWidth=", 10)) {
+ if (action != '\0') {
+ return "Cannot combine '+' or '-' with IconWidth=n";
+ }
d_cfg->icon_width = atoi(&w[10]);
}
else if (!strcasecmp(w, "IconHeight")) {
- d_cfg->icon_height = DEFAULT_ICON_HEIGHT;
+ if (action != '-') {
+ d_cfg->icon_height = DEFAULT_ICON_HEIGHT;
+ }
+ else {
+ d_cfg->icon_height = 0;
+ }
}
else if (!strncasecmp(w, "IconHeight=", 11)) {
+ if (action != '\0') {
+ return "Cannot combine '+' or '-' with IconHeight=n";
+ }
d_cfg->icon_height = atoi(&w[11]);
}
+ else if (!strcasecmp(w, "NameWidth")) {
+ if (action != '-') {
+ return "NameWidth with no value may only appear as "
+ "'-NameWidth'";
+ }
+ d_cfg->name_width = DEFAULT_NAME_WIDTH;
+ d_cfg->name_adjust = 0;
+ }
else if (!strncasecmp(w, "NameWidth=", 10)) {
+ if (action != '\0') {
+ return "Cannot combine '+' or '-' with NameWidth=n";
+ }
if (w[10] == '*') {
d_cfg->name_adjust = 1;
}
@@ -362,10 +414,25 @@
else {
return "Invalid directory indexing option";
}
+ if (action == '\0') {
+ opts |= option;
+ opts_add = 0;
+ opts_remove = 0;
+ }
+ else if (action == '+') {
+ opts_add |= option;
+ opts_remove &= ~option;
+ }
+ else {
+ opts_remove |= option;
+ opts_add &= ~option;
+ }
}
if ((opts & NO_OPTIONS) && (opts & ~NO_OPTIONS)) {
return "Cannot combine other IndexOptions keywords with 'None'";
}
+ d_cfg->incremented_opts = opts_add;
+ d_cfg->decremented_opts = opts_remove;
d_cfg->opts = opts;
return NULL;
}
@@ -418,6 +485,8 @@
new->hdr_list = ap_make_array(p, 4, sizeof(struct item));
new->rdme_list = ap_make_array(p, 4, sizeof(struct item));
new->opts = 0;
+ new->incremented_opts = 0;
+ new->decremented_opts = 0;
return (void *) new;
}
@@ -441,10 +510,31 @@
new->icon_list = ap_append_arrays(p, add->icon_list, base->icon_list);
new->rdme_list = ap_append_arrays(p, add->rdme_list, base->rdme_list);
if (add->opts & NO_OPTIONS) {
+ /*
+ * If the current directory says 'no options' then we also
+ * clear any incremental mods from being inheritable.
+ */
new->opts = NO_OPTIONS;
+ new->incremented_opts = 0;
+ new->decremented_opts = 0;
}
else {
- new->opts = base->opts | add->opts;
+ new->incremented_opts = (base->incremented_opts
+ | add->incremented_opts)
+ & ~add->decremented_opts;
+ new->decremented_opts = (base->decremented_opts
+ | add->decremented_opts);
+ /*
+ * We've got some local settings, so make sure we don't inadvertently
+ * inherit an IndexOptions None from above.
+ */
+ new->opts = ((base->opts | add->opts) & ~NO_OPTIONS);
+ /*
+ * We're guaranteed that there'll be no overlap between
+ * the add-options and the remove-options.
+ */
+ new->opts |= new->incremented_opts;
+ new->opts &= ~new->decremented_opts;
}
new->name_width = add->name_width;
new->name_adjust = add->name_adjust;
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Manoj Kasichainula wrote:
>
> The concept is good, but I have a feeling that these semantics will be
> confusing to users. And the last thing anyone needs is more PRs in the
> bugdb. (I feel your pain Marc, I don't know how you deal with it)
:-)
> It would be easy for a user to see
>
> IndexOptions +IconsAreLinks SuppressLastModified FancyIndexing
>
> And think that this will add IconsAreLinks, SuppressLastModified, and
> FancyIndexing to the list of options.
One of those no-win situations, I think; we already get PRs on
the inheritance issues with IndexOptions as it stands now.
At least the +/- syntax has been around since November 1996.
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Dean Gaudet <dg...@arctic.org>.
But a more drastic change is worth it after 1.3... I've been advocating a
config language cleanup forever ;) We just don't agree on what it'll look
like.
Dean
On Fri, 25 Sep 1998, Manoj Kasichainula wrote:
> On Fri, Sep 25, 1998 at 12:54:34PM -0700, Dean Gaudet wrote:
> > You both realise, of course, that the Options directive works exactly like
> > what Ken's patch does for IndexOptions... right?
>
> Well...
>
> On Fri, Sep 25, 1998 at 02:52:10PM -0500, Me at IO wrote:
> > Of course, the Options directive can be confusing in similar ways, so
> > it might not be a big deal.
>
> yeah, I do. <g> Maybe the syntax for both should be changed. But I
> haven't noticed many complaints about Options, and I don't think that
> a more drastic change is worth the trouble in 1.3.x, so I'll be quiet
> now.
>
> --
> Manoj Kasichainula - manojk at io dot com - http://www.io.com/~manojk/
> "And the moral of today's story is: Never ask what hot dogs are made
> of." - Yakko Warner
>
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Manoj Kasichainula <ma...@io.com>.
On Fri, Sep 25, 1998 at 12:54:34PM -0700, Dean Gaudet wrote:
> You both realise, of course, that the Options directive works exactly like
> what Ken's patch does for IndexOptions... right?
Well...
On Fri, Sep 25, 1998 at 02:52:10PM -0500, Me at IO wrote:
> Of course, the Options directive can be confusing in similar ways, so
> it might not be a big deal.
yeah, I do. <g> Maybe the syntax for both should be changed. But I
haven't noticed many complaints about Options, and I don't think that
a more drastic change is worth the trouble in 1.3.x, so I'll be quiet
now.
--
Manoj Kasichainula - manojk at io dot com - http://www.io.com/~manojk/
"And the moral of today's story is: Never ask what hot dogs are made
of." - Yakko Warner
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Dan Jacobowitz <dr...@false.org>.
On Fri, Sep 25, 1998 at 04:33:55PM -0400, Rodent of Unusual Size wrote:
> > Can we disallow the confusing case, i.e. mixing incremental
> > and absolute in the same statement?
>
> We could, and the user would get an error message if they were
> combined -- but then how do you want to deal with
>
> IndexOptions +IconsAreLinks
> IndexOptions FancyIndexing
> IndexOptions +SuppressLastModified
>
> and stay consistent with the behaviour for all three on the
> same line?
Perhaps check to see if the options are being cleared by an absolute
option before any directive on which they have an effect?
So that, for instance:
IndexOptions FancyIndexing SuppressLastModified
or
IndexOptions +IconsAreLinks
IndexOptions FancyIndexing
would trigger a warning, since it would almost certainly not do what
the user intended.
Dan
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Ben Hyde wrote:
>
> Piffle. Your implying legalese when you use the word.
:-) Which word, precedent or legalese?
> >> Can we disallow the confusing case, i.e. mixing incremental
> >> and absolute in the same statement?
> >
> >We could, and the user would get an error message if they were
> >combined
>
> that's a good thing, right, he wrote junk and we told him so
No, he did't write junk -- he wrote something that may have
result different than he intended. Not the same thing.
Someone may *want* to add a keyword to the end of an existing
IndexOptions statement. I don't buy it. :-) It's also
different from the behaviour of the Options directive with
the same syntactic form.
> IndexOptions FancyIndexing
> IndexOptions SuppressLastModified
> isn't
> IndexOptions FancyIndexing SuppressLastModified
It is with the patch.
> That's why this operation has to use RAW_ARGS now.
No, it uses RAW_ARGS because it never got converted to
ITERATE. (Probably based on the Options code, which is
also RAW_ARGS.) ITERATE is perfectly reasonable for this
directive, in both its original and modified forms. I'll
probably fix that later. I'm on a clean-up-mod_autoindex
kick at the moment.
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Simon Spero <se...@mossflower.oit.unc.edu>.
> Rodent of Unusual Size writes:
> >
> >Feh. A precedent is something that has been done before; i.e.,
>
> Piffle. Your implying legalese when you use the word. My little
> online dictionary say: 1(n) An example that is used to justify similar
Hah - A precedent is someone who receives oral sex from an intern.
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Ben Hyde <bh...@pobox.com>.
Rodent of Unusual Size writes:
>Ben Hyde wrote:
>>
>> An instance is not a precedent, it's an example. Two or three,
>> that's a tradition. A precedent requires a judicious process.
>
>Feh. A precedent is something that has been done before; i.e.,
>that 'precedes' the situation in question. You're thinking
>legalese; go wash your brain out with soap. :->
Piffle. Your implying legalese when you use the word. My little
online dictionary say: 1(n) An example that is used to justify similar
occurrences at a later time. 2(n) (civil law) a law established by
following earlier judicial decisions. Justify, or judicial... in
either case people use it because they like the implication that
something judicious is going on. I find it useful to point out that
one case is hardly judicious.
>> Can we disallow the confusing case, i.e. mixing incremental
>> and absolute in the same statement?
>
>We could, and the user would get an error message if they were
>combined
that's a good thing, right, he wrote junk and we told him so
> -- but then how do you want to deal with
>
> IndexOptions +IconsAreLinks
> IndexOptions FancyIndexing
> IndexOptions +SuppressLastModified
>
>and stay consistent with the behaviour for all three on the
>same line?
So?
IndexOptions FancyIndexing
IndexOptions SuppressLastModified
isn't
IndexOptions FancyIndexing SuppressLastModified
That's why this operation has to use RAW_ARGS now.
- ben
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Ben Hyde wrote:
>
> An instance is not a precedent, it's an example. Two or three,
> that's a tradition. A precedent requires a judicious process.
Feh. A precedent is something that has been done before; i.e.,
that 'precedes' the situation in question. You're thinking
legalese; go wash your brain out with soap. :->
> Can we disallow the confusing case, i.e. mixing incremental
> and absolute in the same statement?
We could, and the user would get an error message if they were
combined -- but then how do you want to deal with
IndexOptions +IconsAreLinks
IndexOptions FancyIndexing
IndexOptions +SuppressLastModified
and stay consistent with the behaviour for all three on the
same line?
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Ben Hyde <bh...@pobox.com>.
Dean Gaudet writes:
>Ken's patch makes things more consistent, your suggestions make things
>less consistent...
Rodent of Unusual Size writes:
>I made an effort to use the same syntax as the only other instance
>of incremental effects we've got -- namely, the Options directive.
>We've already got a precedent for the syntax.
An instance is not a precedent, it's an example. Two or three,
that's a tradition. A precedent requires a judicious process.
Ok, I'll sublimate my reaction to the syntax.
Can we disallow the confusing case, i.e. mixing incremental
and absolute in the same statement?
- ben
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Dean Gaudet <dg...@arctic.org>.
On Fri, 25 Sep 1998, Ben Hyde wrote:
> I'd rather see a more verbose syntax
>
> IndexOptionsEnable IconsAreLinks
> IndexOptionsDisable SuppressLastModified
On Fri, 25 Sep 1998, Manoj Kasichainula wrote:
> The concept is good, but I have a feeling that these semantics will be
> confusing to users. And the last thing anyone needs is more PRs in the
> bugdb. (I feel your pain Marc, I don't know how you deal with it)
You both realise, of course, that the Options directive works exactly like
what Ken's patch does for IndexOptions... right?
Ken's patch makes things more consistent, your suggestions make things
less consistent...
Dean
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Manoj Kasichainula <ma...@io.com>.
On Fri, Sep 25, 1998 at 03:28:02PM -0400, Rodent of Unusual Size wrote:
> One other semantic diference: if a keyword appears
> without a '+' or '-' prefix, it totally clears any
> existing incremental changes and sets the options to
> that keyword. Subsequent keywords with prefixes
> go into the add/remove buckets until the next unprefixed
> keyword appears. That is, these are equivalent:
>
> IndexOptions +IconsAreLinks +SuppressLastModified FancyIndexing
> and
> IndexOptions FancyIndexing
The concept is good, but I have a feeling that these semantics will be
confusing to users. And the last thing anyone needs is more PRs in the
bugdb. (I feel your pain Marc, I don't know how you deal with it)
It would be easy for a user to see
IndexOptions +IconsAreLinks SuppressLastModified FancyIndexing
And think that this will add IconsAreLinks, SuppressLastModified, and
FancyIndexing to the list of options.
Of course, the Options directive can be confusing in similar ways, so
it might not be a big deal.
--
Manoj Kasichainula - manojk at io dot com - http://www.io.com/~manojk/
"Would you die for The One?"
"I wouldn't get pizza for the one. That ain't my job." - J.M. Straczynski
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Ben Hyde wrote:
>
> Nice.
Is that a +1? :-)
> I'd rather see a more verbose syntax
>
> IndexOptionsEnable IconsAreLinks
> IndexOptionsDisable SuppressLastModified
>
> or
> IndexOptionsEdit IconsAreLinks on SuppressLastModified off
Not for 1.3, please. :-)
I made an effort to use the same syntax as the only other instance
of incremental effects we've got -- namely, the Options directive.
We've already got a precedent for the syntax.
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: [PATCH] Add '+' and '-' to IndexOptions keywords
Posted by Ben Hyde <bh...@pobox.com>.
Nice.
I'd rather see a more verbose syntax
IndexOptionsEnable IconsAreLinks
IndexOptionsDisable SuppressLastModified
or
IndexOptionsEdit IconsAreLinks on SuppressLastModified off
This alternative avoids confusion fo
IndexOptions +IconsAreLinks +SuppressLastModified FancyIndexing
and
IndexOptions FancyIndexing
and I just prefer tokens were they really are tokens, i.e.
+foo is RAW.
As an aside, since it would be more features.
It is usually the case that systems like this, (aka style systems
using inheritance with boolean logic combining during inheritance) end
up adding two more operators.
IndexOptionsDisallow IconsAreLinks
IndexOptionsAllow FancyIndexing
I doubt we need Allow :-).
- ben hyde