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

Added graph visualization of notes #921

Closed
wants to merge 10 commits into from
10 changes: 10 additions & 0 deletions resources/templates/dialog/note-network.handlebars
@@ -0,0 +1,10 @@
<div class="dialog">
<h1>{{i18n "dialog.note_network.title" }}</h1>
<p>{{i18n "dialog.note_network.info" }}</p>
<div id="visualize">
<div>
<svg className="svg-content-responsive"></svg>
</div>
</div>
<button id="abort">{{i18n "dialog.button.close"}}</button>
</div>
16 changes: 16 additions & 0 deletions source/main/modules/fsal/fsal-file.js
Expand Up @@ -69,6 +69,7 @@ function metadata (fileObject) {
'ext': fileObject.ext,
'id': fileObject.id,
'tags': fileObject.tags,
'links': fileObject.links,
'type': fileObject.type,
'wordCount': fileObject.wordCount,
'charCount': fileObject.charCount,
Expand Down Expand Up @@ -108,6 +109,7 @@ async function parseFile (filePath, cache, parent = null) {
'ext': path.extname(filePath),
'id': '', // The ID, if there is one inside the file.
'tags': [], // All tags that are to be found inside the file's contents.
'links': [],
'type': 'file',
'wordCount': 0,
'charCount': 0,
Expand Down Expand Up @@ -159,6 +161,7 @@ async function parseFile (filePath, cache, parent = null) {

// Finally, report the tags

Choose a reason for hiding this comment

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

Suggested change
// Finally, report the tags
// Finally, report the tags and links

global.tags.report(file.tags)
global.links.report(file.links)

return file
}
Expand All @@ -171,6 +174,7 @@ function parseFileContents (file, content) {
if (!(/\(.+?\)/.test(idStr))) idStr = `(${idStr})`

let idRE = new RegExp(idStr, 'g')
let linkRE = new RegExp(idStr, 'g')
let linkStart = global.config.get('zkn.linkStart')
let linkEnd = global.config.get('zkn.linkEnd')
// To detect tags in accordance with what the engine will render as tags,
Expand Down Expand Up @@ -249,6 +253,16 @@ function parseFileContents (file, content) {
} else {
file.id = '' // Remove the file id again
}

// Parse links in the file
while ((match = linkRE.exec(mdWithoutCode)) != null) {

Choose a reason for hiding this comment

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

while ((match = linkRE.exec(mdWithoutCode)) != null) works fine, but consider using the new matchAll method because it's more readable and easier to iterate over.

See: String.prototype.matchAll() - JavaScript | MDN.

Note: matchAll is supported natively in Electron v12.0.1 ((Chromium v89, Node v14)) which this projects depends on.

file.links.push({ 'name': file.name.replace(file.ext, ''), 'source': file.id, 'target': match[1] })
}

// Always have atleast one link for identity

Choose a reason for hiding this comment

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

Suggested change
// Always have atleast one link for identity
// Always have at least one link for identity

if (file.links.length === 0) {
file.links.push({ 'name': file.name.replace(file.ext, ''), 'source': file.id, 'target': null })
}
}

async function searchFile (fileObject, terms) {
Expand Down Expand Up @@ -276,8 +290,10 @@ module.exports = {
await updateFileMetadata(fileObject)
// Make sure to keep the file object itself as well as the tags updated
global.tags.remove(fileObject.tags)
global.links.remove(fileObject.links)
parseFileContents(fileObject, content)
global.tags.report(fileObject.tags)
global.links.report(fileObject.links)
fileObject.modified = false // Always reset the modification flag.
cacheFile(fileObject, cache)
},
Expand Down
266 changes: 266 additions & 0 deletions source/main/providers/link-provider.js
@@ -0,0 +1,266 @@
/**
* @ignore
* BEGIN HEADER
*
* Contains: LinkProvider class
* CVM-Role: Service Provider
* Maintainer: Julien Mirval
* License: GNU GPL v3
*
* Description: Handles everything link related that's going on in the app.
*
* END HEADER
*/

/**
* This class manages note's relations on the app. It reads the links on each
* start of the app and writes them after they have been changed.
*/
class LinkProvider {
/**
* Create the instance on program start and initially load the links.
* @param {FSALCache} cache a cache to store links

Choose a reason for hiding this comment

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

The constructor accepts no cache param.

*/
constructor () {
global.log.verbose('Link provider booting up ...')

this._links = []
// The global link database; it contains all links that are used in any of the
// files.
this._globalLinkDatabase = Object.create(null)

// Register a global helper for the link database
global.links = {
/**
* Adds an array of links to the database
* @param {Array} linkArray An array containing the links to be added

Choose a reason for hiding this comment

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

Aside: As a general suggestion about JSDoc comments, I believe it would be better if you could use generic types (e.g. Array<String>) instead of the non-generic version (e.g. Array). This would help others understand the code and (I guess) make the editor provide better hints and type-checking.

This is not really important as Zettlr is migrating to TypeScript anyways.

* @return {void} Does not return.
*/
report: (linkArray) => {
for (let link of linkArray) {
// Create the entry if needed
if (!this._globalLinkDatabase[link.source]) {
this._globalLinkDatabase[link.source] = {}
this._globalLinkDatabase[link.source].name = link.name
this._globalLinkDatabase[link.source].outbound = []
this._globalLinkDatabase[link.source].inbound = []
} else {
this._globalLinkDatabase[link.source].name = link.name
}

// Some links might have no target

Choose a reason for hiding this comment

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

Suggested change
// Some links might have no target
// Some links might have no target (e.g. the null target we set for identity)

if (link.target) {
// Add new links to the entry
this._globalLinkDatabase[link.source].outbound.push(link.target)

// Add a reciproqual inbound entry if needed
if (!this._globalLinkDatabase[link.target]) {
this._globalLinkDatabase[link.target] = {
'name': '',
'outbound': [],
'inbound': []
}
}
// Add only the inbound reference in the target node
this._globalLinkDatabase[link.target].inbound.push(link.source)
}
}

// If we're not booting anymore, update the link database
if (!global.application.isBooting()) {
global.ipc.send('links-database', JSON.parse(JSON.stringify(this._globalLinkDatabase)))
}
},
/**
* Removes the given linkArray from the database, i.e. decreases
* outbound links until zero and then removes the link.
* @param {Array} linkArray The links to remove from the database
* @return {void} Does not return.
*/
remove: (linkArray) => {
for (let link of linkArray) {
// Check if the source is known
if (this._globalLinkDatabase[link.source]) {
// Remove outbound reference from other links
for (const i of this._globalLinkDatabase[link.source].inbound) {
const index = this._globalLinkDatabase[i].outbound.indexOf(link.source)
this._globalLinkDatabase[i].outbound.splice(index, 1)
}

// Remove the link
delete this._globalLinkDatabase[link.source]
}
}

// If we're not booting anymore, update the link database
if (!global.application.isBooting()) {
global.ipc.send('links-database', JSON.parse(JSON.stringify(this._globalLinkDatabase)))

Choose a reason for hiding this comment

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

Suggested change
global.ipc.send('links-database', JSON.parse(JSON.stringify(this._globalLinkDatabase)))
global.ipc.send('links-database', this.getLinkDatabase())

}
},
/**
* Returns the global link database
* @return {Object} An object containing all links.
*/
getLinkDatabase: () => {
return JSON.parse(JSON.stringify(this._globalLinkDatabase))

Choose a reason for hiding this comment

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

We're returning a clone of the "link database" (maybe you want to mention this in the method's documentation comment).

Also, using JSON.parse(JSON.stringify(..)) to clone objects is inefficient especially for large objects or when repeated a lot. If getLinkDatabase() is used a lot, consider using a more efficient cloning approach/function.

},
/**
* Returns the special (= coloured) tags
* @param {String} name An optional name to get one. Otherwise, will return all.
* @return {Array} The special link array.
*/
getSpecialTags: (name) => { return this.get(name) },
/**
* Updates the special links with an array of new ones.
* @param {Array} newlinks An array containing the links to be set.
* @return {Boolean} True if all succeeded, false if at least one failed.
*/
update: (newTags) => { return this.update(newTags) },
/**
* Sync link data from cache. This is called when FSAL is updated
* @param {FSALCache} cache The cache object
*/
sync: (cache) => {
if (!this._initialized) {
this._cache = cache
this._initialized = true
}

this._load()
}
}
}

/**
* Shuts down the service provider
* @return {Boolean} Returns true after successful shutdown
*/
shutdown () {
global.log.verbose('link provider shutting down ...')
this._save()
return true
}

/**
* This function only (re-)reads the links on disk.
* @return {LinkProvider} This for chainability.
*/
_load () {
// We are not checking if the user directory exists, b/c this file will
// be loaded after the ZettlrConfig, which makes sure the dir exists.

if (this._cache) {
// Does the file already exist?
if (this._cache.has('links')) {
this._links = JSON.parse(this._cache.get('links'))
} else {
this._cache.set('links', JSON.stringify([]))
return this // No need to iterate over objects anymore
}

this._checkIntegrity()
}

return this
}

/**
* Simply writes the link data to disk.
* @return {LinkProvider} This for chainability.
*/
_save () {
if (this._cache) {
// (Over-)write the links
this._cache.set('links', JSON.stringify(this._links))
}

return this
}

/**
* This file makes sure all links fulfill certain criteria
*/
_checkIntegrity () {
let nullLink = { 'source': '20200101010101', 'target': '20200101010101', 'name': 'None' }
for (let link of this._links) {
if (typeof link === 'object') {
if (!link.hasOwnProperty('source')) {
link.source = nullLink.source
}
if (!link.hasOwnProperty('target')) {
link.target = nullLink.target
}
if (!link.hasOwnProperty('name')) {
link.name = nullLink.name
}
} else {
// wtf is this? make it go away
this._links.splice(this._links.indexOf(link), 1)
}
}

// Now remove all links that fulfill the "not given" template above completely
for (let link of this._links) {
if (link === nullLink) {

Choose a reason for hiding this comment

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

This expression will always evaluate to false because the nullLink object reference is never inserted into the _links array. What you probably want to test for is whether the link is value-equal* to the nullLink.

As suggested below, you could use a specialized function like this:
const equalLinks = (a, b) => a && b && (a.source === b.source && a.target === b.target && a.name === b.name);

... or use a general function to test for "deep equality" between two objects. See: https://stackoverflow.com/questions/201183
Anyways, I just want to mention this: If you're going to use a generalized function, your test will still fail if the "offending object" contains extra attributes, like { 'source': '20200101010101', 'target': '20200101010101', 'name': 'None', 'someExtraAttribute': 'lol'}.

* Actually there's an npm package named as such: https://www.npmjs.com/package/value-equal

this._links.splice(this._links.indexOf(link), 1)
}
}

return this
}

/**
* Returns a link (or all, if name was not given)
* @param {String} [name=null] The link to be searched for
* @return {Object} Either undefined (as returned by Array.find()) or the tag
*/
get (link = null) {
if (!link) {
return this._links
}

return this._links.find((elem) => { return (elem.source === link.source && elem.target === link.target && elem.name === link.name) })

Choose a reason for hiding this comment

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

Extract the find predicate (we'll be re-using it).

Suggested change
return this._links.find((elem) => { return (elem.source === link.source && elem.target === link.target && elem.name === link.name) })
const equalLinks = (a, b) => a && b && (a.source === b.source && a.target === b.target && a.name === b.name)
return this._links.find((elem) => equalLinks(elem, link))

}

/**
* Add or change a given link.
* @param {String} name The link source's name
* @param {String} source The link source
* @param {String} target The link target
*/
set (name, source, target) {
let link = this.get({ 'name': name, 'source': source, 'target': target })
// Either overwrite or add
if (!link) {
this._links.push({ 'name': name, 'source': source, 'target': target })
}

this._save()

return this
}
Comment on lines +212 to +241

Choose a reason for hiding this comment

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

The set method is not doing what it's claiming in its documentation ("Add or change a given link.").
It is only trying to add an element if it doesn't already exist.

const tmpLink = { 'name': name, 'source': source, 'target': target }
const gotLink = this.get(tmpLink)

if (link) {
  // or maybe use: Object.assign(gotLink, tmpLink)
  // SEE: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign
  // gotLink.name = name // Isn't link.name read-only?
  gotLink.source = source
  gotLink.target = target
} else {
  this._links.push(tmpLink)
}

// ...

More importantly, the get method is not working as expected: get says it will return a Link object where its name attribute matches the passed String argument; however, we are passing (and it is using) a Link-shaped object. Worst, in find, it is testing whether all properties match (name, source, and target) as opposed to just name, and this results in the wrong behavior of returning the link only if it was not changed...

  • Fix: Make the predicate passed to find test only if (elem.name === link.name).

  • Better: Make the get function actually accept only a String argument called name. This will make it make more sense and not force the caller to create temporary objects (like we're currently doing when calling it).

  • Just saying: Having get return different types depending on the input (a single Object/null vs an Array<Object>) feels meh. Maybe another method, getAll, should be responsible for returning all links if the caller intentionally wants them. (Plus it's more descriptive.)


/**
* Updates all links (i.e. replaces them)
* @param {Array} links The new links as an array
* @return {Boolean} Whether or not all links succeeded.
*/
update (links) {
this._links = []
let retVal = true
for (let l of links) {
// Only update correctly set links
if (l.hasOwnProperty('name') && l.hasOwnProperty('source') && l.hasOwnProperty('target')) {
this.set(l.name, l.source, l.target)
} else {
retVal = false
}
}

this._save()

return retVal
}
}

module.exports = new LinkProvider()
9 changes: 9 additions & 0 deletions source/main/zettlr-ipc.js
Expand Up @@ -302,6 +302,11 @@ class ZettlrIPC {
this.send('tags-database', global.tags.getTagDatabase())
break

// Send the global link database to the renderer process.
case 'get-links-database':
this.send('links-database', global.links.getLinkDatabase())
break

// Handle dropped files/folders
case 'handle-drop':
this._app.handleAddRoots(cnt)
Expand Down Expand Up @@ -390,6 +395,10 @@ class ZettlrIPC {
case 'get-tags-database':
return global.tags.getTagDatabase()

// Send the global tag database to the renderer process.
case 'get-links-database':
return global.links.getLinkDatabase()

// Returns the custom CSS's file contents
case 'get-custom-css':
return global.css.get()
Expand Down
5 changes: 5 additions & 0 deletions source/main/zettlr.js
Expand Up @@ -140,6 +140,10 @@ class Zettlr {
this._fsal.on('fsal-state-changed', (objPath, info) => {
// Emitted when anything in the state changes
if (this.isBooting) return // Only propagate these results when not booting

// Sync links when the FSAL ius updated
global.links.sync(this._cache)

switch (objPath) {
// The root filetree has changed (added or removed root)
case 'filetree':
Expand Down Expand Up @@ -221,6 +225,7 @@ class Zettlr {
'dictionary': require('./providers/dictionary-provider'),
'recentDocs': require('./providers/recent-docs-provider'),
'tags': require('./providers/tag-provider'),
'links': require('./providers/link-provider'),
'targets': require('./providers/target-provider'),
'css': require('./providers/css-provider'),
'translations': require('./providers/translation-provider')
Expand Down
8 changes: 8 additions & 0 deletions source/renderer/assets/toolbar/toolbar.json
Expand Up @@ -41,6 +41,14 @@
"title": "toolbar.tag_cloud",
"icon": "tags"
},
{
"role": "button",
"class": "note-network",
"command": "show-note-network",
"content": "{}",
"title": "toolbar.note_network",
"icon": "share"
},
{
"role": "button",
"class": "preferences",
Expand Down