Skip to content

Conversation

zuphilip
Copy link
Contributor

@zuphilip zuphilip commented Mar 7, 2025

These cherry-pick two small commits from #3183.

zuphilip added 2 commits March 7, 2025 16:01
Also fixing potential problems handling keywords with
other tags in general.
Citavi 5 XML.js Outdated
if (rememberTags[noteObject.id]) {
noteObject.tags = rememberTags[noteObject.id];
for (let j = 0; j < rememberTags[noteObject.id]; j++) {
Copy link
Member

Choose a reason for hiding this comment

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

j < rememberTags[noteObject.id] won't work - did you mean to add .length? We should just use for..of here (and elsewhere).

Citavi 5 XML.js Outdated
}
if (pages) {
noteObject.note += "<i>" + pages + "</i>";
}
keywords = ZU.xpathText(doc, '//KnowledgeItemKeywords/OnetoN[starts-with(text(), "' + noteObject.id + '")]');
if (keywords && keywords.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.

Redundant comparison - empty string is falsey

Citavi 5 XML.js Outdated
valueList.push(ZU.xpathText(author, 'Name'));
}
else {
Z.debug("Can't find this id:", id);
Copy link
Member

Choose a reason for hiding this comment

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

debug() doesn't work like console.log(). Need to concatenate the strings.

@zuphilip
Copy link
Contributor Author

Thank you for the code review. I fixed all the points.

@zuphilip zuphilip requested a review from AbeJellinek March 27, 2025 16:52
Citavi 5 XML.js Outdated
Comment on lines 214 to 216
for (let tag of rememberTags[noteObject.id]) {
noteObject.tags.append(tag);
}
Copy link
Member

Choose a reason for hiding this comment

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

There's no array method called append(). push()? Oh, and we can just do this:

Suggested change
for (let tag of rememberTags[noteObject.id]) {
noteObject.tags.append(tag);
}
noteObject.tags.push(...rememberTags[noteObject.id]);

And the same above.

@AbeJellinek
Copy link
Member

@zuphilip: I made the update - would you be able to test? I don't have any Citavi data to test with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants