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.

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

Emit marc:leader as first element #549

Merged
merged 2 commits into from
Jul 12, 2024
Merged

Emit marc:leader as first element #549

merged 2 commits into from
Jul 12, 2024

Conversation

dr0i
Copy link
Member

@dr0i dr0i commented Jul 12, 2024

Fixes #548.

@dr0i dr0i marked this pull request as draft July 12, 2024 12:58
@dr0i dr0i force-pushed the 548-emitMarcLeaderAsFirst branch from 9970929 to df203ee Compare July 12, 2024 12:59
Following hints from IDE:
- remove ignored blank lines in javadoc
- remove unused variables
- inline variable
- simplify expressions that are always true
@fsteeg fsteeg assigned dr0i and unassigned fsteeg Jul 12, 2024
@dr0i dr0i merged commit 0d73713 into master Jul 12, 2024
1 check passed
@dr0i dr0i deleted the 548-emitMarcLeaderAsFirst branch July 12, 2024 13:52
@@ -247,11 +239,12 @@ private static class Encoder extends DefaultStreamPipe<ObjectReceiver<String>> {
private String currentEntity = "";

private boolean emitNamespace = true;
private Object[] namespacePrefix = new Object[]{emitNamespace ? NAMESPACE_PREFIX : EMPTY};
private Object[] namespacePrefix = new Object[]{NAMESPACE_PREFIX};
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change this? Now one has to remember to adjust this line as well should the default for emitNamespace be switched one day.

@@ -353,7 +346,7 @@ else if (!appendLeader(name, value)) {
if (value != null) {
writeEscaped(value.trim());
}
writeTag(Tag.controlfield::close);
writeTag(Tag.controlfield::close, false);
Copy link
Member

Choose a reason for hiding this comment

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

What does this change do? It seems to be ineffective. And also counterintuitive: Why would a close tag have arguments/attributes?

@@ -408,9 +401,20 @@ private void writeFooter() {
* @param str the unescaped sequence to be written
*/
private void writeRaw(final String str) {

Copy link
Member

Choose a reason for hiding this comment

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

Why this empty line?

@@ -432,11 +436,11 @@ private void writeEscaped(final String str) {

private void writeLeader() {
final String leader = leaderBuilder.toString();
if (!leader.isEmpty()) {
if (leaderBuilder.length() > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

String actual = resultCollector.toString();
assertEquals(expected, actual);
}

@Test
public void issue548_failWhenLeaderIsNotFirst() {
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this test? It's already covered by the previous one.

@@ -136,7 +133,6 @@ public void setEmitNamespace(final boolean emitNamespace) {

/**
* Sets the flag to decide whether to omit the XML declaration.
*
Copy link
Member

Choose a reason for hiding this comment

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

The previous formatting looked much clearer to me...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
3 participants