Re: [wip patch] new zsh/attr module

[Mikael Magnusson <mikachu@xxxxxxxxx>]

2009/3/3 Peter Stephenson <pws@xxxxxxx>:
> On Tue, 3 Mar 2009 15:43:29 +0100 (CET)
> Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
>> I've written some basic entries for the builtins, it doesn't seem to show
>> up properly in man zshmodules though, in the summary list at the top it
>> doesn't appear, but it does appear below with the details.
>
> It might be just a dependency problem; it all seems to be there in my case.

Okay, I'll see if it works after some more make cleaning.

>> It's trivial if you know how the standard builtin handling code works,
>> less so otherwise :). I will look into it later. I think I want a -h
>> option for not dereferencing symlinks.
>
> You need a string containing "h" where the builtin is declared and to test
> OPT_ISSET(ops,'h') in the function for the builtin.
>
>> I didn't quite figure out how to best set an array parameter, so for now I
>> still only handle one file, and the zlistattr function sets the whole
>> string with nulls in the parameter, so you have to use ${(0)REPLY} to
>> split it. I looked a little at bin_read since i know read can set an array
>> parameter, but it is probably not very well suited as an easy to
>> understand example since it does so many other things too.
>
> The use of setaparam() in stat.c is probably a good example, it's doing
> something very similar, and there are some hash examples down there, too.
> Remember all the bits come from permanently allocated memory.
>
>> I think I got this right now, but I haven't tested the case where the
>> stuff isn't found.
>
> It looks OK.  I'll see if I can find a Solaris machine left over to try it
> on when I've committed the patch at the bottom.
>
>> It looks like I have to re-metafy the string when using zwarnnam, I pass
>> in the parameter that says the buffer is large enough, since it did
>> contain the metafied version of the string some milliseconds earlier. That
>> should work, right?
>
> It's better to pass in the right length in case of embedded NULs---there
> shouldn't be any since the filesystem calls wouldn't handle them, but it's
> more consistent.  That's an easy change.
>
> Something strange happened with some of the whitespace in the patch so I
> applied various bits by hand.

Ah, I didn't really expect anyone to actually apply it yet but I did
send it through alpine so it's odd that the whitespace was corrupted.

> There are various minor tweaks below: expand the filename in completion in
> case it's got a ~ in it

The completer also needs to check if the file exists before calling
zgetattr, or simply 2> /dev/null I guess.

> reformat the .distfiles (I've been gradually doing
> this to all of them since one file per line is much easier to maintain);
> handle lengths when metafying; don't report errors unless system calls
> returned a negative value; remove an unused variable.

I know the codestyle is a bit questionable but I already check 0 < len
in the first if, so the else will be all negative lengths already.

Another thing that needs to be done (for small values of need) is to
allocate larger buffers in case the actual attributes take up more
space than 256 bytes (or whatever value is a good default first try).

I'll look at the other stuff a little later, thanks for your help so far.

-- 
Mikael Magnusson