Foam: The link generator should take into account ignored folders

[jmg-duarte]

I'm currently trying to build a site for https://github.com/rustype/notes using https://github.com/mathieudutour/foam-gatsby-template.
However, when building locally (for debug purposes) the wiki-link generator starts using the .md files from the _layouts folder.

This should be an easy fix, add an ignore setting as follows:

"foam.edit.linkReferenceDefinitions.ignore": {
  "type": [
    "array"
  ],
  "default": []
}

And filter out files whose path is prefixed with any of the strings present in the array.
From my brief research, I think maybe the following function would be a nice place to filter the files out:

https://github.com/foambubble/foam/blob/532348633b22002088973b4fd608867b50670f9e/packages/foam-vscode/src/features/wikilink-reference-generation.ts#L131-L171

Probably after checking with uniq.
However, the number of operations may be too high in some cases, making the filtering slow, the paths could be split and represented as a tree, then filtering is easy as any branch with a given path can be removed.

[riccardoferretti]

hey @jmg-duarte, thanks for reporting this bug, can you tell me which file and definition is being incorrectly generated? I will try reproduce it to better understand the issue.
A configuration option to exclude some links might be a good solution, but let me first fully understand the problem at hand, thanks!

[jmg-duarte]

I was trying to reproduce the problem to upload it as a branch but I was unable to.
Either way, what happened was:

• I built the site under _layouts using https://github.com/mathieudutour/foam-gatsby-template
• When the link generator kicked in, instead of using my actual markdown files, it would get the rendered .md files present in the _layouts folder

I'd end up with a final link like:

[//begin]: # "Autogenerated link references for markdown compatibility"
[inbox]: _layouts/public/some_really_long_hash/inbox "Inbox"
[//end]: # "Autogenerated link references"

[riccardoferretti]

it sounds like foam was parsing the files inside your _layout directory (which would make sense) and that would cause a conflict with the regular files.

The solution here is telling foam to ignore the files in the _layout directory altogether.
More in general, the solution is to support glob include/exclude patterns in foam.

[jmg-duarte]

Exactly! I am available to help implement the feature

[ingalless]

@jmg-duarte at least to get you started for now, you may want to specify the files.exclude.

For reference, here's our companies files.exclude value in the .vscode/settings.json

  "files.exclude": {
    "**/.git": true,
    "**/.svn": true,
    "**/.hg": true,
    "**/CVS": true,
    "**/.DS_Store": true,
    "**/node_modules": true
  },

[jmg-duarte]

While that works, it sadly hides the files in the file pane

[ingalless]

@jmg-duarte Yeah, that's the only downside! I didn't mind for node_modules, but perhaps your use case is different.

Here's some thoughts around globbing:

• Perhaps .gitignore should be obeyed, but then what if there's a local file that they are hiding from their GitHub for specific reasons?
• We could have a .foamignore?
• Or, a "include"/"exclude" as @riccardoferretti mentioned in a foam specific config file

I don't know how supportive the VS Code files API is of multiple exclude paths. I didn't find anything, but I didn't look particularly hard.

They're the only thoughts I remember having from this PR

[riccardoferretti]

@ingalless might be worth expanding a bit on the role of the files.exclude option in vscode.

By looking at the foam code, we don't filter any file (see https://github.com/foambubble/foam/blob/a34842b95722cdbaa86ce07a5549bbf3947208a9/packages/foam-core/src/bootstrap.ts#L31), so it sounds like those patterns above are used in vscode itself and are totally invisible to anything running inside it, is that right?

The good thing is that we have it out of the box, the bad thing is that it strongly depends on vscode (specifically, our CLI would be unaware of those exclude patterns).

Might be worth bringing this in-house with include/exclude patterns, what do you think?

@jmg-duarte if we move forward with this it would be great if you want to take on the task and make a PR for this!

[jmg-duarte]

I'd say VSCode itself turns a blind eye to files in files.exclude, but they should appear when querying the filesystem through VSCode.

I'd say it is a matter of where you want to place the configuration, in the foam.json file, to be made available to the foam-cli or in VSCode. Or both! Since the VSCode could override the settings if present in the VSCode config.

I'd say we flesh the idea out first and then proceed with the PR, as I am not an experienced JS/TS developer I'd need some guidance around the codebase.

[riccardoferretti]

@jmg-duarte sounds good to me.
The bootstrap function(s) is where the loading of the files happen, so any filtering would be around there.

These could be good places to start:

https://github.com/foambubble/foam/blob/a34842b95722cdbaa86ce07a5549bbf3947208a9/packages/foam-core/src/bootstrap.ts#L44

and

https://github.com/foambubble/foam/blob/a34842b95722cdbaa86ce07a5549bbf3947208a9/packages/foam-vscode/src/extension.ts#L72

and

https://github.com/foambubble/foam/blob/a34842b95722cdbaa86ce07a5549bbf3947208a9/packages/foam-vscode/src/extension.ts#L65

Happy to provide support here or on discord :)

[jmg-duarte]

Just happened with the new graph

I'm putting the image here since the links affect both.
Most of the points are from the site build in _layouts

[riccardoferretti]

yes that makes perfect sense.
The graph uses the foam model, the same used by the generator, so errors should show across features.

That's actually one of the main benefits of consolidating under one model, fix it in one place, fix it everywhere :)

[ingalless]

Yes, sorry. files.exclude instructs the native vscode.workspace.findFiles API to exclude them from a glob.

So, where you've got const files = await workspace.findFiles("**/*");, VSCode will already be excluding those matching the globs defined in files.exclude.

The good thing is that we have it out of the box, the bad thing is that it strongly depends on vscode (specifically, our CLI would be unaware of those exclude patterns).
Might be worth bringing this in-house with include/exclude patterns, what do you think?

These are great points, wholeheartedly agree!

[jmg-duarte]

From my experimentation
https://github.com/foambubble/foam/blob/a34842b95722cdbaa86ce07a5549bbf3947208a9/packages/foam-core/src/bootstrap.ts#L44
Here is too soon, the call returns an empty array.

We can filter here:
https://github.com/foambubble/foam/blob/a34842b95722cdbaa86ce07a5549bbf3947208a9/packages/foam-vscode/src/extension.ts#L65

The code below is a quick experiment that assumes all paths are given from the workspace root.
The reason I extract the path first is because we don't need to iterate the beginning (which is always the same) for each startsWith call.

const files = await workspace.findFiles("**/*").then(f => {
    const root = workspace.workspaceFolders[0].uri.path;
    if (!root) {
      return [];
    }
    const res = f.filter((v) => {
      const path = v.path.substring(root.length);
      console.log(path);
      return !path.startsWith("/_layouts") && !path.startsWith("/assets") && !path.startsWith("/.");
    });
    console.log(res);
    return res;
  });

[riccardoferretti]

that was quick, thanks @jmg-duarte!

Here is too soon, the call returns an empty array.

That doesn't feel right, this is where the legwork should really happen, maybe there is a bug in the code. what's the value of folder?

The code below is a quick experiment that assumes all paths are given from the workspace root.

That's a good starting point, we just need to make the filters more configurable, so that it works with each person's individual setup.

Also, we use https://github.com/isaacs/node-glob in the bootstrap.ts file, it might make sense to reuse it if it satisfies our use case (given that our include/exclude patterns are globs I reckon it will save us some time).

Finally, we need an idea of the relationship/priority between includes/excludes (that is, what happens if a file matches both an include and exclude pattern).
My opinion would be to prioritize excludes, but I am pretty open to other options - feels like a common enough problem that there would be best practices and resources out there on how to think about it (think webpack, babel, .. anything that uses globs).

[jmg-duarte]

That doesn't feel right, this is where the legwork should really happen, maybe there is a bug in the code. what's the value of folder?

The code does not run because config.workspaceFolders returns an empty array.

I also get this Could not read configuration from ~/.foam/config.json. I'm not sure how related it is.

That's a good starting point, we just need to make the filters more configurable, so that it works with each person's individual setup.

I'll add a setting for multiple ignore paths and submit a PR, we can then move discussion there.

Finally, we need an idea of the relationship/priority between includes/excludes (that is, what happens if a file matches both an include and exclude pattern).

I also prefer prioritizing excludes, this errs on the safer way, in general this would help not leaking important information (even though this is not the case).

The glob library seems to be a possible solution, however introducing includes comes with the question, so now we only use files in that folder? Or we keep the others in?
Personally I believe that we can leave includes out, and just add everything (as it is) filtering them out with the excludes.

Question: does the glob library work with file lists? Or does it go directly into the filesystem?
Both https://github.com/isaacs/node-glob and https://github.com/isaacs/minimatch seem too coupled with the filesystem.

However we can glob the excludes, put them in a set and iterate the normally matched files for presence.