davispuh added a comment.

  In D19532#425376 <https://phabricator.kde.org/D19532#425376>, @cullmann wrote:
  
  > No, because defData isn't defined there yet, it's created only later with 
auto defData = DefinitionData::get(d->m_definition);
  >
  > => you can just move that up some lines, or?
  >
  > I actually have taken a short look, perhaps a issue is already that 
"isValid()" is true even if we fail to load anything useful.
  
  
  I'm not sure if we can call `DefinitionData::get(d->m_definition);` when 
`m_definition->isValid` isn't valid.
  So we still would end up with pretty much the same
  
    if (!d->m_definition.isValid()) {
        return State();
    }
    auto defData = DefinitionData::get(d->m_definition);
    if (!defData->isLoaded()) {
        return State();
    }
  
  or wrap it all around
  
    if (d->m_definition.isValid()) {
    
    }
  
  
  
  In D19532#425378 <https://phabricator.kde.org/D19532#425378>, @cullmann wrote:
  
  > F6660164: test.diff <https://phabricator.kde.org/F6660164>
  >
  > This would be my proposed patch + unit test.
  
  
  are you sure we're allowed to call `DefinitionData::get(d->m_definition);` 
when `m_definition->isValid` isn't valid ?

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D19532

To: davispuh, cullmann, dhaumann, vandenoever
Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

Reply via email to