Re: Re: Resource files support (v2)

From: Alexey Pakhunov (lx_uk_at_[hidden])
Date: 2004-06-06 22:12:35

Hi Vladimir,

> 1. The transform and replace methods really don't belong to "res-scanner". It
> would be much more reasonable to add them to the regex module. As for
> replace, I think it is possible to extend regex.replace to accept list, not
> string as the first argument.

Few notes:

I changed regex.transform so it accepts third parameter 'indices' now. If
'indices' ommited transform will work as before. But I had to comment out the
string:

NATIVE_RULE regex : transform ;

otherwise bjam didn't see 3rd parameter. I do not fully understand what it
means. Does bjam support it natively?

Also I placed my 'replace' to regex module as 'replace-list' rule.
'regex.replace' is already used in 'escape-html' rule (print.jam) and its
prototype cannot be changed without changing print.jam. And I think it will be
more efficient to use the former replace here as soon as 'escape-html' performs
several replacements in each string. Or we can write something like a generic
replace rule that would be able to perform several replacements in each string
like:

rule replace ( list * : match : indices * : replacements * )

where 'indices' is a list of paranthethised groups of all successful matches to
replace and 'replacement' is a corresponding list of replacement strings. But I
guess we may postpone it while we really will need it.

BTW: I added two test cases to 'regex.__test__' rule. Both asserts test added
functionality:

assert.result a.h b.h c.h
: transform <a.h> "b.h" <c.h> : <([^>]*)>|\"([^\"]*)\" : 1 2 ;

assert.result "-" "a-b" : replace-list "&" "a&b" : "&" : "-" ;

> 2. Why do you need to replace all double backslashes with signle ones? I'm
> sure you have a reason, I'd just like it to be present as comment

Icons and other includes may referenced as

IDR_MAINFRAME ICON "res\\icon.ico"

so we have to replace double backslashes to single ones. I added this comment to
msvc.jam

> 3. The 'process' rule ends with:
>
> scanner.propagate c-scanner : $(angle) $(quoted) : $(target) ;
>
> Actually, I don't know much about RC files, so don't know if this correct: do
> you really need c-scanner. I realize we can include header which can include
> another header. I wonder if RC files can include other *RC* files?

Indeed. An RC file may include other RC files. So I changed this line:

# Just propagate current scanner to includes, in a hope
# that includes do not change scanners.
scanner.propagate $(__name__) : $(angle) $(quoted) : $(target) ;

Just like in 'c-scanner'.

> Other than this issues, I think you patch can be comitted. Would you be
> willing to send a corrected version?

See attachment. :-)

-- 
Best regards,
Alexey.
 ------=_NextPart_000_006B_01C44C56.6CEA2840 Content-Type: application/octet-stream;
name="rc_support.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
filename="rc_support.diff"
[Attachment content not displayed.] ------=_NextPart_000_006B_01C44C56.6CEA2840--