Before, if you save worksheet and load it, you have see, that after each line have added empty entry. It happens, because command entry after placing result checks existence next entry. If entry don't exist, the command entry add new empty command entry, so worksheet always have empty entry on the end of the worksheet. But when we load entries from file, we haven't empty entry on the end, because we place entries one by one. So each time, when we place command from file, the command after placing result (from file too) add empty entry. It fix this problem by adding check before adding empty entry, that we don't load entries from file.
Details
- Write multiline worksheet
- Save it
- Close it
- Open it
- Checks, that there aren't unnesesary empty entries.
Diff Detail
- Repository
- R55 Cantor
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
A simple
if (m_isLoadingFromFile) return;
in the beginning of Worksheet::appendCommandEntry() should do the same, right? With this we don't need any changes in worksheetentry.cpp and any Worksheet::isLoadingFromFile().
I'd rather call appendEntry(CommandEntry::Type), etc. directly in Worksheet::load(QIODevice*) instead of going via all those append*Entry() functions - this additional indirection (one more function call) is of no benefit here and blocks my proposal for a simple fix for the problem you're addressing here.
I am not sure, that compiler don't optimize it in inline call.
is of no benefit here and blocks my proposal for a simple fix for the problem you're addressing here.
I think your solution simplier, but hachier too. I mean, in appendCommandEntry logic of ignoring is obvicious, but from interface point of view, we sometimes ignore adding command (and only commands) entries (but don't ignore inserting or inserting before). And it's no good, I think, if caller don't know, what entry don't added, because we return nullptr instead valid WorksheetEntry*, and it cause probleams with dereference of nullptr pointer.
I'd rather call appendEntry(CommandEntry::Type), etc. directly in Worksheet::load(QIODevice*) instead of going via all those append*Entry() functions - this additional indirection (one more function call)
I am not sure, that compiler don't optimize it in inline call.
The compiler will most probably inline those calls, yes.
is of no benefit here and blocks my proposal for a simple fix for the problem you're addressing here.
I think your solution simplier, but hachier too. I mean, in appendCommandEntry logic of ignoring is obvicious, but from interface point of view, we sometimes ignore adding command (and only commands) entries (but don't ignore inserting or inserting before). And it's no good, I think, if caller don't know, what entry don't added, because we return nullptr instead valid WorksheetEntry*, and it cause probleams with dereference of nullptr pointer.
This is a very valid argument.