Skip to content

Conversation

@yole
Copy link

@yole yole commented Dec 12, 2024

Closes #327

I haven't received any feedback on the importer so far, so I'm submitting a PR as is. If this is merged, I'd be happy to address future issues as they arise.

@tgrosinger
Copy link
Contributor

Hi @yole,
I'll start reviewing this, but in the mean time could you please put together a few files in Tana that have a variety of link types and markdown syntax, then export them and put them in the tests folder here?

I gave it a try, but couldn't get it to let me in without giving it credit card information :)


const rootNode = this.tanaDatabase.docs.find(n => n.props.name && n.props.name.startsWith('Root node for'));
if (!rootNode) {
this.fatalError = 'Root node not found';
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend throwing an exception and catching it in tana-json.ts instead of storing in a variable.


const workspaceNode = this.nodes.get(rootNode.children[0]);
if (!workspaceNode) {
this.fatalError = 'Workspace node not found';
Copy link
Contributor

Choose a reason for hiding this comment

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

"Root node not found", "Workspace node not found", etc. sound like errors that might not be meaningful to the average user of this plugin. Maybe this should just be logged to console.error and the exception message should be something more generic such as Selected file ${filename} is not a valid Tana export.

this.markSeen(specialNode);
}
else {
this.notices.push('Special node ' + suffix + ' not found');
Copy link
Contributor

Choose a reason for hiding this comment

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

This error also probably will not be actionable for most users.

private nodes: Map<string, TanaDoc>;
private convertedNodes: Set<string> = new Set();
public fatalError: string | null;
public notices: string[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this being consumed?

if (path) {
this.notices.push('Found unconverted node: ' + path);
unconverted++;
if (unconverted == 50) break;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the significancy of 50 unconverted notes? Why are we breaking here?

if (unconverted == 50) break;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity, I would suggest clearing the class-level variables which are only relevant to this call to importTanaGraph. However, I think it would be even better to adjust this function to instead take the ProgressReporter and to convert and save each file to the vault and report progress as it goes along and to not leavea any state behind on the class after being called. Take a look at some of the other format importers to see this pattern.

const properties: Array<[string, string, string]> = [];
this.enumerateChildren(node, (child) => {
if (child.props._docType == 'tuple' && child.children.length >= 2) {
const propNode = this.nodes.get(child.children[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I reading correctly that properties for a node are stored as separate TanaDoc instances?

}

private convertNodeRecursive(node: TanaDoc, fragments: string[], indent: number) {
if (node.props._docType == 'journal') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this is needed here?

}
this.markAssociatedNodesSeen(node);
if (indent == 0 && props.tag) {
fragments.push('#' + props.tag);
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this should go in the tags: [] frontmatter.

}

private convertMarkup(text: string): string {
return text
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this all of the markup allowed in Tana?

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.

Tana importer

2 participants