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

Frankdoc/210 note blocks #216

Merged
merged 7 commits into from
Sep 18, 2024
Merged

Frankdoc/210 note blocks #216

merged 7 commits into from
Sep 18, 2024

Conversation

thomaspj10
Copy link
Contributor

@thomaspj10 thomaspj10 commented Sep 12, 2024

Closes #210

Built upon #209, so that must be merged before this one.

Files:
frankdoc.json

@thomaspj10 thomaspj10 changed the base branch from master to 209/quick-links September 12, 2024 12:40
Base automatically changed from 209/quick-links to master September 13, 2024 14:09
@thomaspj10 thomaspj10 marked this pull request as ready for review September 17, 2024 07:57
Comment on lines 5 to 12
<xs:documentation>First line of documentation.
Second line of documentation.&lt;br&gt;&lt;b&gt;INFO&lt;/b&gt;&lt;p&gt;First line
Second line
Third line&lt;/p&gt;&lt;br&gt;&lt;b&gt;INFO&lt;/b&gt;&lt;p&gt;First line
&lt;pre&gt;&lt;code&gt;&amp;lt;Element&amp;gt;
&amp;lt;Param /&amp;gt;
&amp;lt;/Element&lt;/code&gt;&lt;/pre&gt;&lt;/p&gt;&lt;br&gt;&lt;b&gt;TIP&lt;/b&gt;&lt;p&gt;First line&lt;/p&gt;&lt;br&gt;&lt;b&gt;WARNING&lt;/b&gt;&lt;p&gt;First line
Second line&lt;/p&gt;&lt;br&gt;&lt;b&gt;DANGER&lt;/b&gt;&lt;p&gt;First line&lt;/p&gt;</xs:documentation>
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Poeh, ik ga er van uit dat jij gecontrolleerd hebt dat dit er mooi/leesbaar uit ziet?

Copy link
Contributor Author

@thomaspj10 thomaspj10 Sep 17, 2024

Choose a reason for hiding this comment

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

image

Het ziet er best prima uit, maar line breaks zijn een beetje deceiving. Als je meerdere paragrafen wilt moet je <p></p> gebruiken. Dit is ook hoe het in de rest van de frankdoc werkt, dus niet persee bijzonder.

@nielsm5 nielsm5 merged commit 337ff70 into master Sep 18, 2024
2 checks passed
@nielsm5 nielsm5 deleted the frankdoc/210-note-blocks branch September 18, 2024 08:36

Choose a reason for hiding this comment

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

Waarschijnlijk kan jij er niets aan doen, maar waarom heet dit ding DocWriterNew? Ik zie nergens *Old ..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ik las daar net toevallig een stukje over in de code:

Quote:

 * This package contains a set of model classes that is used by {@link org.frankframework.frankdoc.DocWriterNew} to generate the
 * XML configuration schema used by Frank developers. Please note that {@link org.frankframework.frankdoc.DocWriterNew} is
 * presently not used; this class is under development.

Dit is nu natuurlijk niet het geval meer. Er zijn een aantal classes die zo gek heten dus ik zal er een issue voor aanmaken.

Comment on lines 764 to 784
private void addDocumentationFrom(XmlBuilder element, FrankElement frankElement) {
if(version == XsdVersion.STRICT) {
String elementDescription = frankElement.getDescription();
if(! StringUtils.isBlank(elementDescription)) {
addDocumentation(element, elementDescription);
if (!StringUtils.isBlank(frankElement.getDescription())) {
StringBuilder description = new StringBuilder(frankElement.getDescription());

if (!frankElement.getNotes().isEmpty()) {
description.append("<br>");
}
for (Note note : frankElement.getNotes()) {
description.append("<br><b>")
.append(note.type().name())
.append("</b>")
.append("<p>")
.append(note.value())
.append("</p>");
}

addDocumentation(element, description.toString());
}
}
}

Choose a reason for hiding this comment

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

Ik moest echt even drie keer kijken wat hier nou gebeurt. Doordat description afhanekelijk van parameters wordt gemuteerd wordt dit denk ik voor mij wat onduidelijk. Ik zou denk ik op zoiets uitkomen:

	private void addDocumentationFrom(XmlBuilder element, FrankElement frankElement) {
		String elementDescription = frankElement.getDescription();
		
		if (version == XsdVersion.STRICT && !StringUtils.isBlank(elementDescription)) {
			if (frankElement.getNotes().isEmpty()) {
				addDocumentation(element, elementDescription);
			} else {
				StringBuilder description = new StringBuilder(elementDescription);
				description.append("<br>");

				frankElement.getNotes()
					.forEach(note -> description.append(formatNote(note)));

				addDocumentation(element, description.toString());
			}
		}
	}

	private String formatNote(Note note) {
		return String.format("<br><b>%s<b/><p>%s</p>", note.type().name(), note.value());
	}

Hiermee return je wanneer je niets te doen hebt hier en is door de else branch en formatNote wmb duidelijk wat er gebeurt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Goeie suggestie, maar het issue is al gemerged, dus ik kan die wijzingen hier nu niet doorvoeren..

This pull request was closed.
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.

Implement a tag for info/tip/warning/danger blocks
3 participants