[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
Alejandro-FA opened this issue Jan 25, 2024 · 2 comments · Fixed by #1264
Closed

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

Alejandro-FA opened this issue Jan 25, 2024 · 2 comments · Fixed by #1264

Comments

@Alejandro-FA
Copy link
Contributor

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
Copy link
Contributor Author
Alejandro-FA commented Jan 25, 2024 •
edited
Loading

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
Copy link
Contributor Author

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);
}

@Alejandro-FA Alejandro-FA mentioned this issue Jan 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix indentation of empty flow sequences and add test Alejandro-FA/yaml-cpp
1 participant
@Alejandro-FA