Dekel Tsur <[EMAIL PROTECTED]> writes:
| | void LyXTextClass::readOutputType(LyXLex & lexrc)
| | {
| | keyword_item outputTypeTags[] = {
| | { "docbook", DOCBOOK },
| | { "latex", LATEX },
| | { "linuxdoc", LINUXDOC },
| | { "literate", LITERATE }
| | };
|
| Why isn't outputTypeTags declared static?
I though a bit wrong about this and tought we could save some
runtime-footprint.
|
| | pushpophelper pph(lexrc, outputTypeTags, LITERATE);
|
| It is not a good idea to use the tag LITERATE because someone might change
| enum OutputType by adding a new tag after LITERATE, but he may forget to
| change the line above.
Agree.
| A better idea is to use sizeof(outputTypeTags)/sizeof(keyword_item),
| (another option is to add a line "OutputType_LAST = LITERATE" to
| enum OutputType, and then use OutputType_LAST instead of LITERATE)
I think I'd prefere the sizeof()/sizeof() method. That is the one we
use elsewhere.
| | int le = lexrc.lex();
| | switch(le) {
| | case LyXLex::LEX_UNDEF:
| | lexrc.printError("Unknown output type `$$Token'");
| | return;
| | case LATEX:
| | case LINUXDOC:
| | case DOCBOOK:
| | case LITERATE:
| | outputType_ = static_cast<OutputType>(le);
| | break;
| | default:
| | lyxerr << "Unhandled value " << le
| | << " in LyXTextClass::readOutputType." << endl;
| |
| | break;
| | }
|
| The code here isn't very pretty.
| The value of 'le' can be either an error code from LyXLex::lex() namely
| either LyXLex::LEX_UNDEF, LEX_FEOF or LEX_DATA,
| and otherwise, it is either LATEX,LINUXDOC,DOCBOOK or LITERATE
| so the code can be as follows:
|
| switch(le) {
| case LyXLex::LEX_UNDEF:
| lexrc.printError("Unknown output type `$$Token'");
| return;
| case LyXLex::LEX_FEOF:
| case LyXLex::LEX_DATA:
| lyxerr << "Unhandled value " << le
| << " in LyXTextClass::readOutputType." << endl;
| default:
| outputType_ = static_cast<OutputType>(le);
| }
Hmm... yes in this case I think you are right. But if we also thing
about concistency between this and the other parsing funcitons your
way of doing it is not applicable to all of them.
Lgb
