Am Freitag, 29. September 2006 10:01 schrieb Ozgur Ugras BARAN:
> Thanks a lot for reviewing the code. I will be more careful about
> formatting. Here are some some answers:
> 
> - What I mean with the nested command support is something like:
>          \command {\command{content}}
>  An example is basic index command : \index { entry @{\myformat
> {entry}} }. I see that this is not necessary for the moment, but,
> well... I am just speculating. :)

I think we should not allow arbitrary LaTeX in these parameters, only 
normal text.

> - Indeed implementing map was easier than implementing the multimap.
> But I couldn't imagine if a parameter can apply multiple times or not.
> Then I tried to stay in the safe side. Speculating again, take the
> index command.. An index entry like
>  \index {aaa!bbb!ccc}
>  can be handled as:
> LatexCommand
> command_name index
> add_index_entry aaa
> add_index_entry bbb
> add_index_entry ccc
> end_inset
> As I said, just a speculation...

This is a specialization, but I think we should keep it generic and simple. 
InsetCommandParams would just store a number of parameters, and the LaTeX 
command generated from then would simply be
\command{param1}[optparam2][optparam3]{param4} etc.

> - I have extended the old constructor for two reasons. First, I use sc 
for
> nomencl implementation, so that it should be extended. Second, I had
> to change all initiations of insetcommandparams in the lyx code, but I
> was not sure whether my implementation will be accepted or not. I
> assume that It is accepted. Then we can go back the old constructor for 
the moment, since I have already handled nomencl with new constructor. In 
future, I will clean all traces of old constructor and we can remove it 
from the code.
> 
> - Please be careful that, on the code I submitted the line that calls 
scan command is commented out. Therefore, For the moment it is not 
backward compatible. we need two lines of code there.. I can submit the 
diff..

Not needed, I saw that.

> - I would create a small struct that contains token, value and -maybe- 
order. Then a vector of the struct should be enough since its order is 
preserved. Well, I like simpler things :)..

Yes, that would be simple. In this case order is not needed in the struct.

> Another point is, what you are asking is actually in the code. the tokens 
(text_after, citation) is kept in the vector<string> key_.. Using this key 
you can fill in multimap. If you just iterate over multimap you preserve 
the order anyhow. I just did not commit anything about key_, since I am 
not sure how to introduce this.  
> First option is to add an static array, that contains all key strings:
>            string key_ = {"text_after","citation", ...};
> the second option is use a setKeys(vector<string keys>) function to 
introduce keys for a particular inset type.

I do not see why we need to know the keys explicitly. Can you please 
explain? IMO it is sufficient to just for a key, e.g. the cite inset would 
ask for "citation". This key must of course appear in the file, and if it 
does not then asking for it would result in an error.

> - Sorry again for altered whitespace. My editor did that. I tried to 
correct it after I finished with the code, but I guess it was not 
enough :).

Then you should get a decent editor that does not fight you in maintaining 
indentation etc, but helps.

> - There is something nasty in the svn code, it is now not possible to 
alter the insets. I couldn't trace the differences, but the behavior 
changed around revision 149xx. Therefore I cannot commit a clean diff for 
the nomencl support. I will send it after I managed to run it properly.

Sure you can. If the current code does not work for you, then you can 
always do an

svn update -r 14900

. If you then generate a diff with svn diff it will be the right thing. But 
of course it would be even better to find the problem in current snv :-)


Georg

Reply via email to