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