Skip to content

Fix output of info parser / info unparser in debug#806

Merged
stevedlawrence merged 1 commit intoapache:mainfrom
OWL-Varun:daffodil-2488-debugger-correct-result-for-infoParser-infoUnparser
Jul 1, 2022
Merged

Fix output of info parser / info unparser in debug#806
stevedlawrence merged 1 commit intoapache:mainfrom
OWL-Varun:daffodil-2488-debugger-correct-result-for-infoParser-infoUnparser

Conversation

@OWL-Varun
Copy link
Contributor

When the user is in debug mode with the subcommand parse, the ouput for
info unparser will now display correctly.
When the user is in debug mode with the subcommand unparse, the output
for info parser will now display correctly.

When the user is in debug mode while calling the parse or unparse
subcommand, we want to make sure that the info printed to the screen for
parser and unparser are accurate. This is done by comparing the
subcommand and the info argument. The "info parser" command prints the
parser information only when the parser subcommand is utilized. The "info
unparser" command prints the unparser information only when the unparse
subcommand is utilitzed.

DAFFODIL-2488

Copy link
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, suggest we remove some tests

}
}

@Test def test_CLI_Debugger_parse_unparser_not_available_no_breaks(): Unit = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure a separate test for no breaks is needed. I don't think this provides any real additional correctness. And integration tests are quite slow so we want to minimize unnecessary tests.

In fact, this "no breaks" test is probably the correct way to write the test, with the expects immediately after the associated sendLine. I would suggest removing the above test and just keeping this one (with no_breaks removed).

Same suggestion below for the parser not available tests.

debugPrintln("parser: not available")
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought, I wonder if there is value in adding a new info processor command, which would print out either the parser or unparser depending on which is available (i.e. the current behavior of the InfoParser and InfoUnparser commands)?

And then the "not available" logic would move to the individual parser/unparse commands. E.g.

class InfoParser ... {
  def act(...) = {
    state match {
      case _: PState => debugPrintln("%s: %s".format(name, processor.toBriefXML(2)))
      case _ => debugPrintln("%s: not available".format(name))
    }
  }
}

InfoUnparser would be the same but would match on UState instead of PState.

We could also deprecate info parser/unparser and just have the info processor command if we wanted. I'm not sure there's much benefit from having both when they do basically the same thing.

If there's consensus that this seems reasonable, we can either add it to this PR, or just open a new feature ticket to implement some other day. I have no strong preference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest we integrate this as is, but that next change should be another ticket, but might as well get it done right away while all fresh in mind.

Why would I suggest this?... because the world seems full of interrupts of context to me right now.

Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

When the user is in debug mode with the subcommand parse, the ouput for
info unparser will now display correctly.
When the user is in debug mode with the subcommand unparse, the output
for info parser will now display correctly.

When the user is in debug mode while calling the parse or unparse
subcommand, we want to make sure that the info printed to the screen for
parser and unparser are accurate. This is done by comparing the
subcommand and the info argument. The "info parser" command prints the
parser information only when the parser subcommand is utilized. The "info
unparser" command prints the unparser information only when the unparse
subcommand is utilitzed.

DAFFODIL-2488
@OWL-Varun OWL-Varun force-pushed the daffodil-2488-debugger-correct-result-for-infoParser-infoUnparser branch from ed0bcf2 to 3f6b501 Compare June 30, 2022 13:23
@stevedlawrence stevedlawrence merged commit 4cac2b0 into apache:main Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants