jbeder / yaml-cpp

A YAML parser and emitter in C++
MIT License
4.91k stars 1.77k forks source link

Empty sequence is not emitted correctly when used as a key in flow style #1263

Closed Alejandro-FA closed 5 months ago

Alejandro-FA commented 5 months ago

Brief description

An empty sequence, used as a map key and emitted in flow style, adds two white-spaces at the front. This behavior is not present for non-empty sequences. It is also not present for empty sequences written as a map value instead of a map key.

Steps to reproduce

std::ofstream output_file("test.yml");
if (!output_file) throw std::runtime_error("Could not open file.");

YAML::Emitter emitter;
emitter << YAML::BeginMap;
emitter << YAML::Key << YAML::Flow << YAML::BeginSeq << 1 << 2 << 3 << YAML::EndSeq;
emitter << YAML::Value << 1;
emitter << YAML::Key << YAML::Flow << YAML::BeginSeq << YAML::EndSeq;
emitter << YAML::Value << 2;
emitter << YAML::Key << 3;
emitter << YAML::Value << YAML::Flow << YAML::BeginSeq << YAML::EndSeq;
emitter << YAML::EndMap;

output_file << emitter.c_str() << '\n';
output_file.close();

Expected output

[1, 2, 3]: 1
[]: 2
3: []

Current output

[1, 2, 3]: 1
  []: 2
3: []
Alejandro-FA commented 5 months ago

I have been able to spot the source of the undesired indent space:

void Emitter::EmitEndSeq() {
  if (!good())
    return;
  FlowType::value originalType = m_pState->CurGroupFlowType();

  if (m_pState->CurGroupChildCount() == 0)
    m_pState->ForceFlow();

  if (m_pState->CurGroupFlowType() == FlowType::Flow) {
    if (m_stream.comment())
      m_stream << "\n";
    m_stream << IndentTo(m_pState->CurIndent()); // This is the problematic line
    if (originalType == FlowType::Block) {
      m_stream << "[";
    } else {
      if (m_pState->CurGroupChildCount() == 0 && !m_pState->HasBegunNode())
        m_stream << "[";
    }
    m_stream << "]";
  }

  m_pState->EndedGroup(GroupType::Seq);
}

m_stream << IndentTo(m_pState->CurIndent()); seems to only take effect if the current line is empty. If the current line already contains some elements, it seems to be simply ignored. Removing the line seems to solve the issue. However, I suspect that it is there for some reason that eludes me. Any idea if it is save to simply remove the line?

Alejandro-FA commented 5 months ago

I decided to take a closer look and run the tests under the test/yaml-cpp-tests target. Indeed, it is not correct to simply remove the m_stream << IndentTo(m_pState->CurIndent()); line, since it breaks some of the tests. The solution that I have found is to not add the indentation if the original Flow type was FlowType::Flow and m_pState->HasBegunNode() is false:

void Emitter::EmitEndSeq() {
  if (!good())
    return;
  FlowType::value originalType = m_pState->CurGroupFlowType();

  if (m_pState->CurGroupChildCount() == 0)
    m_pState->ForceFlow();

  if (m_pState->CurGroupFlowType() == FlowType::Flow) {
    if (m_stream.comment())
      m_stream << "\n";
    if (originalType == FlowType::Block || m_pState->HasBegunNode()) // New line added
      m_stream << IndentTo(m_pState->CurIndent());
    if (originalType == FlowType::Block) {
      m_stream << "[";
    } else {
      if (m_pState->CurGroupChildCount() == 0 && !m_pState->HasBegunNode())
        m_stream << "[";
    }
    m_stream << "]";
  }

  m_pState->EndedGroup(GroupType::Seq);
}