On Sun, 16 Jul 2006, Dominik Vogt wrote:
On Fri, Jul 14, 2006 at 05:30:03PM -0500, fvwm-workers wrote:
CVSROOT: /home/cvs/fvwm
Module name: fvwm
Changes by: griph 06/07/14 17:30:03
Modified files:
. : ChangeLog NEWS
fvwm : expand.c fvwm.1.in
libs : Strings.c Strings.h
Log message:
* add variable filters
* make backslash escape character within $[...] expressions
If you write such patches that are monitored by several developers
it would be nice to explain what you are doing and how. We have
to be very careful with big changes in the parsing code because it
has the potential to break everything. Personally I want to
proofread such changes.
I'll do so in the future. The changes are quite isolated. Most additions
are done in new functions. Appart from that the start of the filters are
registered in the parsing of the variable name and sent to the expansion
functions. They have some code added for when the filters are non-NULL.
With filters==NULL the same code as before is executed. I can understand
that you want to proofread the changes.
The syntax I quoted that Mikhael suggested has not really been
thought about a lot. It's just an ad-hoc idea. Now that you
implemented it, I think it has several general issues:
The best way to see limitations in a (simple) system is to implement it
and play around with it. It's not mush work put down into the
implementaion. I mainly needed the single and quote filter myself when
the ranged positional parameters changed expansion and thought I'd give
it a try.
* There is currently no way to quote a literal string, which is a
very important feature. It would allow users to write monsters
like this more easily:
+ I PipeRead 'cd $HOME/.fvwm/palettes; /bin/ls *.pal | sed -e
\"s:\\(.*\\)\\(\\.pal\\):+ \\\\&\\1 Function SetPalette \\1\\2:\"'
I see your point. I aslo thought about the gettext expansion, and possible
problems of changing the ':' meaning.
* The syntax is too verbose. The strings you have to write eat
up too much space. Also, the name "filters" is misleading.
In almost all syntax in fvwm is verbose. Why should this be an exception?
Except for the fact that the way it is done now makes the filters to be
parsed several times. I'm sure it's possible to optimize that by
preparsing the filter-string and invent some easier to deal with
representation. However I didn't want to try to do that since I thought
that the patch might have to be reverted anyway.
I suggest to follow zsh's example:
$[(flags)name]
Where "name" is the parameter name itself and "flags" describes
the modifications to apply to the expansion process.
Thinking of this compared to the one currently implemented I must say that
this has several advances:
* it will most likely not break anything existing (at least I've never
seen an environment variable containing braces.
* it's easier to preparse the flags since the order doesn't matter.
Flags:
q<style>[<n>] (quote)
Quote the string <n> times using the given <style>.
<style> can be one of
q quote normally for use as a token
mi quote for use as a menu item label
mt quote for use as a menu title
l (literal)
Quote a literal string, i.e. everything up to the closing
brace.
r0 (recursive expansion off, default)
r1 (recursive expansion on)
Enable or disable recursive expansion ($[$[X]])
Note: If the flags l and r1 are present, recursive
expansion takes place first, and the result is used as
literal input for further processing - it is *not*
interpreted as a variable name.
s[<n>] (split)
Split string into individual tokens (by removing <n> levels
of quotes and whitespace). <n> defaults to 1.
t (tokenize)
Treat the resulting string as a single token (Useful in
ranges).
Examples:
With X="foo `bar`", $0="0", $1="1", ... (without the
quotes):
$[(qq)X]
--> 'foo `bar`'
$[(s1qq)X]
--> 'foo' 'bar'
$[(lqq2)foo `bar`]
--> "'foo `bar`'"
$[(lr1)X]
--> foo `bar`
$[(qq)1-3]
--> '1' '2' '3'
$[(tqq)1-3]
--> '1 2 3'
Maybe flags should be separated by colons to allow more and
longer flag names like $[(literal:qq)foo bar]
Thinks that are not good in the current patch:
* I think we shouldn't have different styles of quoting filters.
Just quote: should be enough. This is because quoting is just
meant to be used *internally by fvwm*. And fvwm does not care
wether tokens are quoted by ", ', ` or \. Fvwm could just
choose the type of quotes that has the shortest result.
A string quoted this way can *not* be passed to a shell for
example.
What I did regarding filters was to try implement the ones in todo-vars
based on what was given there.
* It should be possible to add multiple levels of quotes in a
single filter (usefull in above example).
If doining it with flags that's a good feature. With the current patch
it's possible to apply multiple filters, but as noted above it's not very
optimized for handling that sort of thing.
* The menu: filter should be split into two. In menu titles, the
characters ^, @, % and * have to be quoted (by doubling them).
In menu item labels, %, & and * have to be quoted. In either
case, the TAB character has to be quoted too.
The current menu-filter is quite useless. It's based on the todo-vars file
and nothing else.
* The glob: filter seems to promise something it can not live up
to. It can *not* be used for shell command quoting. Other
than that I see no purpose in that specific filter. It should
be removed.
I don't even know what the glob filter is supposed to do. I just took it
from todo-vars.
* The noquote: filter should be removed.
This filter is useful for overrideing fvwm defaults. If for instance one
wants the name of a window echoed without any quoting.
* In constructs like $[$[X]], if X is "quote:foo", the result
should be the contents of the variable "quote:foo", not the
quoted result of the variable "foo". (I did not check how the
code qorks now).
I take it that you mean "foo:quote". In that case the result will be the
variable "'foo'" that is it will perform quoting within the recursive
expansion.
Please take this as a base for discussion, *not* as instructions
to change the code now!
So, should the current patch to expand.c and the manpage be reverted until
the system has been decided on? The functions in Strings.[ch] would still
be usful.
/Viktor
