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

Create a proposal for the "new note" fallback chain #683

Closed

Conversation

movermeyer
Copy link
Collaborator

@movermeyer movermeyer commented Jun 17, 2021

My work on allowing templates in the user's home directory,

  • combined with this question about having a global setting for default note creation,
  • combined with Foam's inadequate handling of the edge case where no workspace is open

Has forced me to consider what the complete new note creation fallback chain ought to be. I realized that I really need to nail it down so that I could enforce consistency between all of Foam's methods of creating notes.

Here's what I've managed to come up with. Feel free to criticize it in any way.

It's probably easiest to view the rendered version of this document.

Let me know what you think so I can get the commands all switched over to use it.

@riccardoferretti

Copy link
Collaborator

@riccardoferretti riccardoferretti left a comment

Choose a reason for hiding this comment

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

left some thoughts, let me know if I understood this correctly

* Consistency: with the behaviour of the existing template commands.
* Flexibility: Users could use variables like `$FOAM_TITLE` (and eventually other snippet variables) in the path.

### Phase 4: Match `SAME_AS_ACTIVE_NOTE` behaviour
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if Phase 4, 5, 6 would be better described asa options of Phase 3. The same thing you do in Phase 2.

* This behaviour matches `<workspace>/.foam/templates/new-note.md` behaviour
* **Option B**: Relative to `foam.newNoteDirectory`
* I suspect is what you'd expect if you've already defined `foam.newNoteDirectory`
* **Option C**: Both. Relative to `foam.newNoteDirectory`, only when `foam.newNoteDirectory` is defined. Otherwise, relative to `<home_dir>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you follow the foam.newNoteDirectory resolution from Phases 3, 4, 5, 6, we could at this level simply go with Option B, which would basically include Option C.

mmm.. not sure I am making sense.

Basically, a relative filepath would be matched against the resolved foam.newNoteDirectory.

foam.newNoteDirectory is resolved using the cases from Phase 3, 4, 5, 6 - and those are a superset of Phase 2 -> Option C.

Let me know if this makes sense

Copy link
Collaborator Author

@movermeyer movermeyer Jun 21, 2021

Choose a reason for hiding this comment

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

@riccardoferretti

While Phase 6 is <home_dir>, it is really included just to handle the extreme edge case of having neither a file, nor a workspace open. If we resolve foam.newNoteDirectory from Phase 3-6, I expect that it would almost always get resolved by Phase 4, which changes the behaviour from what is currently described.

(i.e. Phase 4 "gets in the way", and is probably not the behaviour we want (creating a note relative to whatever you happen to have open)


I think you raise a good point to think about: Perhaps the resolution could be separated into two parts:

  1. Resolving from the config files, falling back to foam.newNoteDirectory
  2. Resolving foam.newNoteDirectory

I'll give it some more thought, but I'm not convinced yet.


Potential changes to Phase 2 ("Option D")

I think the desired fallback behaviour of a relative path in Phase 2 might be:

Similar to being relative to Phases 3, 5, 6 (note: not Phase 4):

  • Similar to Phase 3: Relative to foam.newNoteDirectory
  • Similar to Phase 5: Relative to Workspace
  • Similar to Phase 6: Relative to <home_dir>

This would just affect Phase 2 with a relative path. In other cases, we'd we continue as described, doing Phase 3-6 (including 4)

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • In phase 3, how does a user set SAME_AS_ACTIVE_NOTE and WORKSPACE_ROOT?
    are those presented as "options" or "variables?

  • an interesting things about phase 2 resolution is that you could have variables that only work in certain context (e.g. WORKSPACE_ROOT), I guess in that case they simply don't get expanded or, better, we can error out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@riccardoferretti

I'm not proposing the addition of SAME_AS_ACTIVE_NOTE and WORKSPACE_ROOT into Foam, just equivalent behaviour as those.

I've reworded Phase 4 and 5 to be clearer in that regard.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I understand, my question was about how the user could be deliberately set foam.newNoteDirectory to "same as active note" or "workspace root", is this a supported case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmmm... 🤔 ... good question...


foam.newNoteDirectory is defined as matching the spec used for filepath in the templates.

Once I convince VS Code to give extensions better snippet APIs, we'll be able to use snippet variables in these filepaths, including:

  • TM_DIRECTORY: The directory of the current document
  • WORKSPACE_FOLDER: The path of the opened workspace or folder

(Which work today in the rest of the snippets)

So at that point, users would be able to use those to get these behaviours.

  • SAME_AS_ACTIVE_NOTE -> "foam.newNoteDirectory": "$TM_DIRECTORY/$FOAM_TITLE"
  • WORKSPACE_ROOT -> "foam.newNoteDirectory": "$WORKSPACE_FOLDER/$FOAM_TITLE"

@movermeyer movermeyer force-pushed the filepath_fallback_path branch from 8687678 to 26c29b2 Compare June 27, 2021 11:04
@movermeyer
Copy link
Collaborator Author

movermeyer commented Jul 13, 2021

@riccardoferretti Here's a reformulation. Let me know what you think.

This is the same logic as described in the original proposal, but fills in the edge cases that were glossed over in the original proposal:

  • What to do in the case of finding a relative filepath in a template (uses "Option D")
  • What to do in the case of finding a relative filepath in foam.newNoteDirectory

Round 2

In order to create a note, we need 3 components:

  1. Filename: The filename to use
  2. Directory: The directory to create the note in
  3. Contents: The contents of the note

Together, the Filename and Directory form a Filepath (i.e. a Uri)

  • Templates give you Contents.
  • An absolute filepath template attribute gives you both Filename and Directory
  • A relative filepath template attribute gives you only Filename.

So the question is how to determine the other components in each case.


This is somewhat precise to communicate, so I found that English was more verbose than I wanted.
So I decided to write it as pseudocode instead. I'll rewrite the English in this PR once we've agreed on the logic.

Pseudocode disclaimers:

  • The names are arbitrary to get the point across and will be changed to whatever makes sense.
  • Please ignore the hidden circular dependency of fallbackFilename
    • fallbackFilename will probably depend on the content
    • The actual code will not be structured like this; I tried writing the pseudocode in a more representative way but it obscured the behaviour I'm trying to convey.

Pseudocode

function newNoteInformation(fallbackContent: string, fallbackFilename: string) {
  let contents: string, filepath: Uri;

  // Try to get the results from the workspace's template
  [content, filepath] = loadFilepathFromTemplate('<workspace>/.foam/templates/new-note.md');

  // Note: We don't want to use the `filepath` from the <home_dir> template
  // if we've already loaded the content from the <workspace> template.
  if (!content) {
    // Try to get the results from the <home_dir>'s template
    [content, filepath] = loadFromTemplate('<home_dir>/.foam/templates/new-note.md');
  }

  // Resolve relative paths
  // Note: If the filepath came from a template, we don't want to fall through to
  // activeTextEditorDirectory, which is why this block exists separate from the one below.
  if (filepath && !isAbsolute(filepath)) {
    const directory = workspaceDirectory || homeDirectory;
    filepath = Uri.joinPath(directory, fallbackFilename);
  }

  // Fallback to loading filepath from the config setting
  filepath ||= resolveSnippetVariablesIn(configuration.get('foam.newNoteDirectory'));

  // Fallback further and resolve relative paths
  if (!filepath || !isAbsolute(filepath)) {
    const directory = activeTextEditorDirectory || workspaceDirectory || homeDirectory;
    filepath = Uri.joinPath(directory, fallbackFilename);
  }

  return [content, filepath];
}

@riccardoferretti
Copy link
Collaborator

riccardoferretti commented Jul 15, 2021

I think this makes a lot of sense, I have slightly reworded it (the main difference I think is around areWeInWorkspace).
Is this semantically what you meant?

function newNoteInformation(fallbackContent: string, fallbackFilename: string) {
  let contents: string, filepath: Uri;

  // Try to get the results from the workspace's template
  [content, filepath] = loadFilepathFromTemplate('<workspace>/.foam/templates/new-note.md');

  // Note: We don't want to use the `filepath` from the <home_dir> template
  // if we've already loaded the content from the <workspace> template.
  if (!content) {
    // Try to get the results from the <home_dir>'s template
    [content, filepath] = loadFromTemplate('<home_dir>/.foam/templates/new-note.md');
  }

  // if no template is found, use the default content 
  content ||= defaultContent;

  // if filepath is not defined, check variable
  filepath ||= resolveSnippetVariablesIn(configuration.get('foam.newNoteDirectory'));
  // if filepath variable is not defined, use default
  filepath ||= fallbackFilename

  const directory = areWeInWorkspace
    ? workspaceDirectory || homeDirectory
    : activeTextEditorDirectory || workspaceDirectory || homeDirectory;

  filepath = isAbsolute(filepath) ? filepath : Uri.joinPath(directory, filepath);

  return [content, filepath];
}

@movermeyer
Copy link
Collaborator Author

Is this semantically what you meant?

@riccardoferretti We're in agreement with the first bunch of lines:

function newNoteInformation(fallbackContent: string, fallbackFilename: string) {
  let content: string, filepath: Uri;

  // Try to get the results from the workspace's template
  [content, filepath] = loadFilepathFromTemplate('<workspace>/.foam/templates/new-note.md');

  // Note: We don't want to use the `filepath` from the <home_dir> template
  // if we've already loaded the content from the <workspace> template.
  if (!content) {
    // Try to get the results from the <home_dir>'s template
    [content, filepath] = loadFromTemplate('<home_dir>/.foam/templates/new-note.md');
  }

  // if no template is found, use the default content 
  content ||= fallbackContent;

But it seems that you have changed the semantics of the latter portion with the refactor.
It seems that your changes you may have been trying to collapse the two resolution chains into the same block:

  • workspaceDirectory || homeDirectory; ("Option D")
  • activeTextEditorDirectory || workspaceDirectory || homeDirectory; ("Phases 4, 5, 6")

Which is laudable. But I don't think what is written is what we want.

const directory = areWeInWorkspace
    ? workspaceDirectory || homeDirectory
    : activeTextEditorDirectory || workspaceDirectory || homeDirectory;

Which resolution/fallback chain to use doesn't depend on whether we are in a workspace or not, but rather whether the filepath came from a template filepath metadata attribute.

I didn't find a nicer way to know whether the filepath came from a template other than having the two blocks there.
(I'm not sure it's a big deal to find a better way right now in the pseudocode, as long as we're aligned on the logic)

@riccardoferretti
Copy link
Collaborator

Yeah, the later part is what I am struggling with. I think I understand your comment, let me see if I can rewrite it in such a way that it feels clearer to me an aligned with your thoughts. I think part of the challenge is that we are sometimes using content and filepath as proxies for "have we found the template" and collapsing decisions paths (so it's semantically correct, but possibly harder to parse).

Let me try again with a few points.

  • Point A: Is this fixing the issue from my previous proposal?
function newNoteInformation(fallbackContent: string, fallbackFilename: string) {
  let contents: string, filepath: Uri;

  // Try to get the results from the workspace's template
  [content, filepath] = loadFromTemplate('<workspace>/.foam/templates/new-note.md');

  // Note: We don't want to use the `filepath` from the <home_dir> template
  // if we've already loaded the content from the <workspace> template.
  if (!content) {
    // Try to get the results from the <home_dir>'s template
    [content, filepath] = loadFromTemplate('<home_dir>/.foam/templates/new-note.md');
  }

  // if no template is found, use the default content 
  content ||= defaultContent;

  // Resolve relative paths
  // Note: If the filepath came from a template, we don't want to fall through to
  // activeTextEditorDirectory, which is why this block exists separate from the one below.
  if (filepath) {
    if (!isAbsolute(filepath)) {
      const basedir = workspaceDirectory || homeDirectory;
      filepath = Uri.joinPath(basedir, fallbackFilename);
    }
  } else {
    // Fallback to loading filepath from the config setting
    filepath = resolveSnippetVariablesIn(
      configuration.get('foam.newNoteDirectory') ?? fallbackFilename
    );

    if (!isAbsolute(filepath)) {
      const basedir = activeTextEditorDirectory || workspaceDirectory || homeDirectory;
      filepath = Uri.joinPath(basedir, fallbackFilename);
    }
  }

  return [content, filepath];
}
  • Point B: I have consolidated loadFromTemplate and loadFilepathFromTemplate . Was there a reason to separate the two that I am missing?

  • Point C: resolveSnippetVariablesIn is only used when filepath comes from configuration (as opposed to template). I guess this is because in the template it is automatically expanded?

@riccardoferretti
Copy link
Collaborator

hi @movermeyer what's your thinking around this PR/proposal?

**Changes from the initial proposal:**

* Renamed `foam.newNoteDirectory` to `foam.rootDirectory`
  * It's not a filepath, but a directory.
  * It can no longer accept snippet variables
    * I can't imagine a scenario where you'd want snippet variables in this
    * But if such a scenario comes about, we could add it then.
* Relative path fallback in Phase 2 no longer considers the active note's directory (as it did in Option D; now only the workspace and homedir)
  * Assertion: A user will never want to define a path relative to whatever file they happen to have open.
* All references to `<home_dir>` (esp. Phase 6) changed to `foam.rootDirectory ?? <home_dir>`
@movermeyer
Copy link
Collaborator Author

Point B: I have consolidated loadFromTemplate and loadFilepathFromTemplate. Was there a reason to separate the two that I am missing?

Yeah, looking back, I think that was just a typo. 😅

Point C: resolveSnippetVariablesIn is only used when filepath comes from configuration (as opposed to template). I guess this is because in the template it is automatically expanded?

Yeah.


@riccardoferretti The silver lining of coming back to this after a month is that I can see the proposal from a different perspective.

I've given it another review and adjusted it after asking myself "What value does each one of these Phases solve?"

I've added another commit to update the Markdown proposal document to reflect these changes, which I also describe below.


Round 3

Goals

  • 100% coverage: Foam should always know where to / have a place to create a note
  • Sensible UX: Simplicity and consistency
  • Progressive UX: Users start with sensible defaults, then can customize as they get more sophisticated.

foam.newNoteDirectory -> foam.rootDirectory

Something that I had to remind myself of, because I was always getting it wrong.
foam.newNoteDirectory is not a direct equivalent of filepath from the templates.
Rather, it allows users to set a directory other than the home directory to be the default location.

This is a useful distinction. In the future, if there are other files needed to configure Foam (other than templates) then they can also use this fallback.
If we defined foam.newNoteDirectory to be template specific, then we'd have to add a new setting, even though in 90%+ of cases you'd want them to be the same.

To remind myself and make it clearer, I've renamed foam.newNoteDirectory to foam.rootDirectory.


Justification for each Phase

Phase 1: Use <workspace>/.foam/templates/new-note.md

Relative: Allows users to have customized everything in a given workspace.
Absolute: Allows uses to have their notes for a project inside another repository.

Phase 2: Use ${foam.rootDirectory ?? <home_dir>}/.foam/templates/new-note.md

Allows users to define templates that can be used globally.

Absolute filepath: Allows for a global directory/filepath pattern.
Relative:

  • to the current workspace (~Phase 5)
    • This allows you to always store notes in a notes subdirectory of each project, etc.
  • then to foam.rootDirectory ?? <home_dir> (~Phase 6) in the edge case where no workspace is open.
    • Mostly here just to have a reasonable fallback in this edge case. It's kinda as if your Foam root directory is a default workspace.

Phase 3: Use the directory defined in foam.rootDirectory

Allows users to define a global directory to hold all Foam-related stuff (e.g., a second brain repository).

Phase 4: Use the directory of the active editor

Default when nothing else is defined. Opens in the same directory as the current file.

Phase 5: Use the workspace root

A reasonable fallback in the edge case where there isn't a file open.

Phase 6: Use foam.rootDirectory ?? <home_dir>

A reasonable fallback in the edge case where there isn't a workspace open.
It's kinda as if your Foam root directory is a default workspace.

In code

function newNoteInformation(defaultContent: string, fallbackFilename: string) {
  let contents: string, filepath: Uri;

  // Phase 1: Try to get the results from the workspace's template
  [content, filepath] = loadFromTemplate('<workspace>/.foam/templates/new-note.md');
  // The workspace's templates relative filepath can only be relative to the workspace
  if (filepath && !isAbsolute(filepath)) {
    filepath = Uri.joinPath(workspaceDirectory, fallbackFilename);
  }

  const foamRootDirectory = configuration.get('foam.rootDirectory') ?? homeDirectory

  // Note: We don't want to use the `filepath` from the <home_dir> template
  // if we've already loaded the content from the <workspace> template.
  if (!content) {
    // Phase 2: Try to get the results from the <home_dir>'s template
    [content, filepath] = loadFromTemplate(Uri.joinPath(foamRootDirectory, '.foam/templates/new-note.md'));
  }

  // if no template is found, use the default content
  content ||= defaultContent;

  // Resolve relative filepaths from Phase 2
  if (filepath && !isAbsolute(filepath)) {
    const basedir = workspaceDirectory || foamRootDirectory ?? homeDirectory;
    filepath = Uri.joinPath(basedir, fallbackFilename);
  }

  // Phase 3: Use `foam.rootDirectory`
  // Note: We don't use `?? homeDirectory` here, since that would always resolve and Phase 4 would never occur
  const foamRootDirectoryWithoutFallback = configuration.get('foam.rootDirectory')
  if (!filepath && foamRootDirectoryWithoutFallback){
    filepath = Uri.joinPath(foamRootDirectoryWithoutFallback, fallbackFilename);
  }

  // Phase 4
  if (!filepath && activeTextEditorDirectory){
    filepath = Uri.joinPath(activeTextEditorDirectory, fallbackFilename);
  }

  // Phase 5/6
  if (!filepath){
    const basedir = workspaceDirectory || foamRootDirectory;
    filepath = Uri.joinPath(basedir, fallbackFilename);
  }

  return [content, filepath];
}

Changes from the initial proposal

  • Renamed foam.newNoteDirectory to foam.rootDirectory
    • It's not a filepath, but a directory.
    • It can no longer accept snippet variables
      • I can't imagine a scenario where you'd want snippet variables in this
      • But if such a scenario comes about, we could add it then.
  • Relative path fallback in Phase 2 no longer considers the active note's directory (as it did in Option D; now only the workspace and homedir)
    • Assertion: A user will never want to define a path relative to whatever file they happen to have open.
  • All references to <home_dir> (esp. Phase 6) changed to foam.rootDirectory ?? <home_dir>

Progression of Foam usage maturity

  • When users first start using Foam, they have set up nothing. They skip to Phase 4.
  • They then might decide they want to set a global location where they store their notes (e.g., a second brain repository)
    • They then define foam.rootDirectory, which means that they use Phase 3.
  • Then they might get into templates, which allow the fine-grain control of Phases 1 and 2:
    • At first, they'll probably set them up in foam.rootDirectory to control their notes globally (Phase 2)
    • For finer grain control, they Set up overrides of the global templates on a per-workspace level (Phase 1)

Copy link
Collaborator

@riccardoferretti riccardoferretti left a comment

Choose a reason for hiding this comment

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

thinking of the root dir is definitely the right way to go, made me think of a couple more things that I have left in the comments


This document will guide the development direction that each note-creating-command will eventually be modified to follow.

Note that throughout this doc, `foam.rootDirectory` is used as a placeholder name to denote a new Foam setting that we would add.
Copy link
Collaborator

Choose a reason for hiding this comment

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

renaming this foam.rootDirectory makes a world of difference, totally changes the mental model around it - and I agree 100% it's the right way to go

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that mindset, I wonder whether we can say we always have:

  • foam.rootDirectory, which is the root directory for Foam, and is used e.g. to read config, templates, ...
  • foam.notesRootDirectory, which is the root directory when scanning for notes, or computing relative paths, ...
  1. if we are in a VS Code workspace:
    • foam.rootDirectory is <workspace>
    • and foam.notesRootDirectory defaults to <foam.rootDirectory> (but can be defined in the config - <foam.rootDirectory>/.foam/config.json or ~/.foam/config.json)
  2. if we are not in a VS Code workspace:
    • foam.rootDirectory is <home_dir>
    • and foam.notesRootDirectory defaults to <foam.rootDirectory>/foam (but can be defined in <foam.rootDirectory>/.foam/config.json)

Once we can assume that these variables are always defined, things should get simpler with the resolution.
E.g. relative paths are always relative to foam.notesRootDirectory.

Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Full reply is here: #683 (comment)


Q: Why is there a different defaulting behaviour for foam.notesRootDirectory between the "workspace" and "global" levels?
If there's a need to add a foam subdirectory, shouldn't it be applied at both levels for consistency? (Aside/nit: default to notes instead of foam?)

* Progressive UX: Users start with sensible defaults, then can customize as their usage of Foam gets more sophisticated.

## Proposal: "New note" path resolution

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add the pseudo-code from the PR (or a mermaid flow chart) in this doc, to summarize the flow (the sections can still be kept to go into deeper detail and share the rationale around each step).

* A relative path in `filepath` in `<workspace>/.foam/templates/new-note.md` should be relative to the workspace.
* **Justification:** Allows users to have customized templates in a given workspace.
* An absolute path in `filepath` in `<workspace>/.foam/templates/new-note.md` should just use that absolute path.
* **Justification:** Allows uses to have their notes for a project inside another repository.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
* **Justification:** Allows uses to have their notes for a project inside another repository.
* **Justification:** Allows users to have their notes for a project inside another repository.

@movermeyer
Copy link
Collaborator Author

Thoughts?

@riccardoferretti I think I've convinced myself that splitting foam.rootDirectory into two concepts makes sense.
I've written a stream of consciousness below that walks through my thinking.
I'd appreciate some additional context to answer some of the questions I raise.

I'm out of time right now, but I will definitely need to do another round of changes to the document.
If you find time before I get to it, have a read and let me know if you think I'm headed in the wrong direction with the vague statements I have here.
Once I update the document, it should be a lot more clear/concrete.


I'm missing the context around .foam/config.json. Why not use VSCode's settings.json?
I went looking in the code briefly and see that this exists:

// TODO this is still to be improved - foam config should
// not be dependent on vscode but at the moment it's convenient
// to leverage it

But I don't know the "Why?"


Q: Would foam.rootDirectory also be configurable, or always be implicit?

  • If so, where would it be configured? In the VSCode settings.json?
    • I had assumed that foam.rootDirectory would be defined in VSCode settings.json. Now that I know about config.json, I'm somewhat confused.

Q: Why do you need a foam.notesRootDirectory? How is it actually defined?

Taking a guess as to why you introduced foam.notesRootDirectory:

  • In "Phase 3" as written today, you wouldn't have the flexibility to have your notes created in a subdirectory of foam.rootDirectory.
    • I would agree, that probably most users would not want their notes directly in that root directory.
    • If this is your concern, perhaps there is another way to do it that avoids having another arbitrary filepath?
      • Perhaps Phase 3 could be defined as: "in a notes subdirectory under foam.rootDirectory"???
        • If the user at this level of Foam usage maturity wants more customization, they graduate to filepath in templates, which allows them to define either absolute paths, or paths relative to foam.rootDirectory.
        • Phases 5 and 6 would also change to use a notes subdirectory also.

Conceptually, I think I want:

  • a) there are two level to find Foam stuff: "workspace" and "global"/"root"
  • b) the two places are identical in how they work: use the same file structures, semantics, etc.

This feels like we need a table of user stories to be able to verify the behaviour is reasonable in all scenarios:

In a workspace, file open No workspace, file open In a workspace, No file open No workspace, No file open
Scenario A: First-time user New note created beside the open file (Phase 4) New note created beside the open file (Phase 4) New note created inside notes directory (Phase 5) New note created in ~/notes (Phase 6)
Scenario B: Has set explicit foam.rootDirectory New note created in foam.rootDirectory (Phase 3) New note created in foam.rootDirectory (Phase 3) New note created in foam.rootDirectory (Phase 3) New note created in foam.rootDirectory (Phase 3)
Scenario C: Has global template with no filepath New note created in foam.rootDirectory (Phase 3) New note created in foam.rootDirectory (Phase 3) New note created in foam.rootDirectory (Phase 3) New note created in foam.rootDirectory (Phase 3)
Scenario D: Has global template with relative filepath New note created in path relative to foam.rootDirectory (Phase 2) New note created in path relative to foam.rootDirectory (Phase 2) New note created in path relative to foam.rootDirectory (Phase 2) New note created in path relative to foam.rootDirectory (Phase 2)
Scenario E: Has global template with absolute filepath New note created in that absolute path (Phase 2) New note created in that absolute path (Phase 2) New note created in that absolute path (Phase 2) New note created in that absolute path (Phase 2)
Scenario F: Has workspace template with no filepath ????!!!!! New note created in foam.rootDirectory (Phase 3) N/A ????!!!!! New note created in foam.rootDirectory (Phase 3) N/A
Scenario G: Has workspace template with absolute filepath New note created in that absolute path (Phase 1) N/A New note created in that absolute path (Phase 1) N/A
Scenario H: Has workspace template with relative filepath New note created in path relative to workspace (Phase 1) N/A New note created in path relative to workspace (Phase 1) N/A

I've filled it in with the behaviour as written in the proposal document today (commit feaf353).

Phase 3 as currently defined is perhaps too "strong": As soon as they define an explicit foam.rootDirectory, (Scenario B) then all new notes created get created there.

  • There is no more relative note creation behaviour until you graduate to templates with filepaths.
  • It's unclear to me if this is what a user would want. It's consistent with the way I've been modeling user maturity.

It's kind of weird that Scenario F ends up falling back to foam.rootDirectory. IMO, it should probably behave in a similar way to Scenario C and create the note in the workspace.
IMO, foam.rootDirectory defines the global Foam root, and should only be used if we are resolving things at a "global" level, not a workspace level.


This all seems to indicate that it might make sense to define a foam.notesRootDirectory:

  • If absolute, use that.
  • If relative, it is relative to either the workspace or the global root (foam.rootDirectory)
  • Defaults to /notes

Rename it to something else; it isn't specific to the "root" level, so perhaps drop that?: foam.notesDirectory?

I really have to write up another draft of the proposal with this new concept, and redo the scenarios chart.

I'm out of time for now, but dropping this here to have it recorded.

@riccardoferretti
Copy link
Collaborator

I'm missing the context around .foam/config.json. Why not use VSCode's settings.json?

Foam is conceived to work as a stand-alone library, and with various clients, amongst which VS Code.
Initially there was also a Foam CLI, which has been sunset to focus instead on the VS Code extension.
That being said, and I am increasingly considering it, there is a serious case to make that this is overcomplicating a lot of aspects, from development (this is the reason for foam-core and foam-vscode), to configuration (to your point about .foam/config.json), and code (see uri.ts in foam-core).

@riccardoferretti
Copy link
Collaborator

riccardoferretti commented Sep 10, 2021

Q: Why do you need a foam.notesRootDirectory? How is it actually defined?

It is partially for the reason you stated, that users might want to have their Foam repo to be in a subdir (e.g. docs/ as it is for Foam itself). Right now the workaround is to use include and exclude patterns.

The main reason why I introduced the concept though was to create a reliable base for workspace relative paths.
From my comment above:

  1. if we are in a VS Code workspace:
    • foam.rootDirectory is <workspace>
    • and foam.notesRootDirectory defaults to <foam.rootDirectory> (but can be defined in the config - <foam.rootDirectory>/.foam/config.json or ~/.foam/config.json)
  2. if we are not in a VS Code workspace:
    • foam.rootDirectory is <home_dir>
    • and foam.notesRootDirectory defaults to <foam.rootDirectory>/foam (but can be defined in <foam.rootDirectory>/.foam/config.json)

Once we can assume that these variables are always defined, things should get simpler with the resolution.
E.g. relative paths are always relative to foam.notesRootDirectory.

In the non VS Code workspace case above, I feel we still need the concept of root for notes (e.g. for computing relative paths), but to make that the user dir sounds weird. So we would need to define one.

Perhaps Phase 3 could be defined as: "in a notes subdirectory under foam.rootDirectory"???

I think this is basically the same concept but without giving such path a name?

Let me know if it's still unclear, to me it feels right but I might be missing something or not making the point come across (which is itself a yellow flag)

@riccardoferretti
Copy link
Collaborator

Regarding your scenarios, I think this is where things become much simpler:

  1. if path is relative, create the note relative to the notesRootDirectory
  2. if path is absolute, create the note at the given path

The only open question I can see is if someone who has opened a file (and not a folder) should be considered to be in a workspace that has notesRootDirectory equal to the base path of the file, or not.

Thoughts?

@riccardoferretti
Copy link
Collaborator

Rename it to something else; it isn't specific to the "root" level, so perhaps drop that?: foam.notesDirectory?

Good point, foam.notesDirectory sounds good to me

This all seems to indicate that it might make sense to define a foam.notesRootDirectory:

If absolute, use that.
If relative, it is relative to either the workspace or the global root (foam.rootDirectory)

if relative, it is relative to foam.rootDirectory, wherever it comes from - right

Defaults to /notes

The default for me changes if we are in a workspace (at which point it defaults to foam.rootDirectory) vs not (in which case it defaults to ./notes, which is <foam.rootDirectory>/notes, which by default is ~/notes)

Makes sense?

@movermeyer
Copy link
Collaborator Author

@riccardoferretti I think its accurate to say you've been trying to model <foam.rootDirectory> (and foam.notesDirectory) as a variable that gets resolved based on the current environment.
I'm probably missing something, but I don't think that model is flexible enough.

For example, if we always resolve <foam.rootDirectory> to be <workspace> when within a workspace, and everything else resolves relative to <foam.rootDirectory>, then there is no way to "inherit" settings from a global configuration.
This means that scenarios like the following are not possible:

  • In a new workspace, you would not be able to use global note templates, since <foam.rootDirectory> always resolves to <workspace>

That is not how I am modelling it. I am treating it as a user setting (like any other) that controls the location of the global-level Foam configuration.


I know that I started this issue discussing the "new note" fallback chain, but I think this topic is actually more general: How Foam should resolve its configuration settings?.

As such here is my new proposal for resolving all Foam configuration values.

In summary, each configuration setting will be resolved by falling through a hierarchy of "scopes", stopping at the first scope that resolves a value for the setting.

local -> client -> global -> defaults

  • local is "local" to the currently open editor:

    • <local_dir> = <workspace_dir> ?? dirname(vscode.window.activeTextEditor)
    • If a workspace is open, <local_dir> = <workspace_dir>
    • If no workspace is open, but a file is open, <local_dir> = <workspace_dir>
    • If no workspace is open, and no file is open, then the local scope does not exist / doesn't resolve a value for any setting. (edge case)
    • Check for configuration in <local_dir>/.foam. If found, use that. If it doesn't exist, then the local scope does not exist / doesn't resolve a value for any setting.
  • client uses the configuration mechanism of the Foam client to resolve settings values.

    • e.g., settings.json in VSCode
    • (Perhaps environment variables for a CLI client?)
  • global uses the location defined by the setting <foam.globalConfigDirectory>

    • Check for configuration in <foam.globalConfigDirectory>/.foam. If found, use that. If it doesn't exist, then the global scope does not exist / doesn't resolve a value for any setting.
    • Note that this is somewhat recursive, since the location of <foam.globalConfigDirectory> itself resolves the value of a setting. We'll talk about that a bit later.
  • defaults which contains the default values for each setting.

Pseudocode:

function resolveConfigSetting(settingName: string) -> Optional<string> {
  return scopes.find(scope => scope.resolveConfigSetting(settingName))
}

Claim: I believe this fallback chain unifies the behaviour of configuration value resolution and allows the flexibility that allows advanced users to customize their workflows, while maintaining sensible defaults throughout the user maturation process. It provides a consistent interface in the code for accessing any config value, and maintains the ability to make use of a client's native configuration mechanisms.

The case of new note filepath resolution

Finding any note templates follows the same fallback path as any setting, first checking for the template body in:

  • the local scope (which looks in <local_dir>/.foam/templates)
  • the client scope (in the case of VSCode, as currently imagined, it cannot resolve a template body)
  • the global scope (which looks in <foam.globalConfigDirectory>/.foam/templates)
  • the default scope, which provides the default template body

Aside: If there are "just any other setting", we should probably give the template bodies official settings names: <foam.newNoteTemplate> and <foam.dailyNoteTemplate>

(Undefined: What if foam.newNoteTemplate is defined in the local scope .foam/config.json and .foam/templates/new-note.md also exists? Or do we define local and global as ignoring foam.newNoteTemplate in config.json? Anyways, that's going to be an edge-case detail in the local scope, and probably not worth talking about more right now)

Once you have the template body, you parse it to pull out the filepath template metadata value. This brings us back to the original aim of this issue.

  • If the filepath is absolute, use that.
  • If the filepath is relative, make it relative to the resolved value of the <foam.notesDirectory> setting.
  • If filepath is not defined, use the resolved value of the <foam.notesDirectory> setting.

The <foam.notesDirectory> setting

The value of the foam.notesDirectory setting always resolves to an absolute filepath, regardless whether the user defines it as a relative path or not.

  • local returns the value of <foam.notesDirectory> if it is defined in <local_dir>/.foam/config.json.
  • client returns the value of <foam.notesDirectory> if it is defined in the client (e.g., In VSCode, the value of foam.notesDirectory defined in settings.json).
  • global returns the value of <foam.notesDirectory> if it is defined in <foam.globalConfigDirectory>/.foam/config.json.
  • defaults returns dirname(vscode.window.activeTextEditor) ?? <workspace_dir> ?? <foam.globalConfigDirectory>
    • This is roughly Phases 4,5,6 of the original writeup
    • (Note that this is the opposite resolution order of <local_dir> so I couldn't use that.)

If the resolved value is relative, then it is relative to <local_dir> ?? <foam.globalConfigDirectory>; resolve it as an absolute filepath.

  • ?? <foam.globalConfigDirectory> is just to handle the edge case where neither workspace nor file is open, so there is no <local_dir>.

The <foam.globalConfigDirectory> setting

The location used by the global scope is determined by the resolved value of the <foam.globalConfigDirectory> setting.
When resolving settings in the global scope, it must first resolve the <foam.globalConfigDirectory> setting

  • global is unable to resolve a value for <foam.globalConfigDirectory>, by definition (otherwise, it would be recursive).
  • defaults returns <home_dir>

Aside: Edge case silliness

This setting was imagined to be defined by users in the client scope, it could technically be defined at the local scope.
This would we a strange case, and it is not expected to be useful. But for completeness, it would technically work.
A workspace could define <foam.globalConfigDirectory> to be some other location than what is typically used.
It's not clear to me if there is ever a scenario where this would be desired.
Technically, this would allow for per-workspace customization that could be overridden by the client scope. 🤷‍♂️

User Maturation

  • When users first start using Foam, they have set up nothing. When they create a new note:
    • <foam.newNoteTemplate> resolves to the default note template by defaults scope.
    • <foam.notesDirectory> resolves to the result of dirname(vscode.window.activeTextEditor) ?? <workspace_dir> ?? <foam.globalConfigDirectory> by defaults scope.
      • ~Phases 4,5,6
    • The note is created at <foam.notesDirectory>/$FOAM_TITLE.md
  • If a user wants to create a location for all notes related to a workspace, they set foam.notesDirectory in the local scope
  • If a user wants to have a global note directory, they set an absolute path for foam.notesDirectory in the global scope (e.g., "foam.notesDirectory": "/src/foam_data/notes")
    • This would be done in <foam.globalConfigDirectory>/.foam/config.json which by default is <home_dir>/.foam/config.json
  • If a user wants to have notes always created in a subdirectory local to the current editor, they set an relative path for foam.notesDirectory in the global scope (e.g., "foam.notesDirectory": "notes/")
  • If a user wants to define local note templates, they do so in <local_dir>/.foam/templates
  • If a user wants to define global note templates, they do so in <foam.globalConfigDirectory>/.foam/templates
  • If a user wants to override foam.notesDirectory in the templates (or supply a path relative to foam.notesDirectory), they can by setting filepath.

Limitations

No way to specific a relative path that is relative to anything other than <local_dir>

Note that since relative <foam.notesDirectory> are always resolved relative to <local_dir>, that there is currently no way to say define a relative <foam.notesDirectory> that resolves to <foam.globalConfigDirectory>/notes.
For example, relative filepath metadata, with a relative <foam.notesDirectory> defined in the global scope, resolves to <local_dir>/<foam.notesDirectory>/<filepath>, with no current way to instead define it to resolve to <foam.globalConfigDirectory>/<foam.notesDirectory>/<filepath>.
This is consistent with the current behaviour of filepath:

filepath | The filepath to use when creating the new note. If the filepath is a relative filepath, it is relative to the current workspace.

I don't think this is an important problem, but in case it is...

  • As a workaround, one can provide an absolute filepath to <foam.notesDirectory> in the global scope.
  • If this proves to be unacceptable, then perhaps in the future we'd expose a $FOAM_GLOBAL_CONFIG_DIRECTORY snippet variable. This would allow the filepath to define an absolute filepath instead of a relative one.
  • Alternatively (cleaner, solving at the right level?), perhaps we'd allow settings to refer to the resolved values of other settings; foam.notesDirectory = <foam.globalConfigDirectory>/notes
    • Of course, this could result in recursions, etc, which would be much more complicated (or even Turing complete?!) and should probably be avoided.

No way to specify dynamic subdirectories relative to a global directory in <foam.notesDirectory>

Migration of existing settings

All existing configuration settings for Foam would be migrated to use this pattern.
This would provide a unified interface/mechanism to get all configuration values.

Foam: Show Resolved Configuration Settings

Having multiple scopes for configuration allows for the flexibility users desire, but can sometimes leave you guessing where a configuration value came from.
To aid in debugging configuration, we could add a Foam: Show Resolved Configuration Settings which prints each setting's resolved value, as well as the scope that resolved it.

* `foam.dailyNoteTemplate`: "# $CURRENT_YEAR..." (truncated) (from `global` scope, found in `/Users/michael/foam_data/.foam/templates/daily_note.md`)
* `foam.globalDirectory`: "/Users/michael/foam_data" (from `client` scope, found in VSCode's `settings.json`)
* `foam.newNoteTemplate`: "# $FOAM_TITLE..." (truncated) (from `local` scope, found in `.foam/templates/new_note.md`)
* `foam.notesDirectory`: "notes" (from `local` scope, found in `.foam/config.json`)
* `foam.openDailyNote.titleFormat`: "yyyy-mm-dd" (from `client` scope, found in VSCode's `settings.json`)

@movermeyer
Copy link
Collaborator Author

movermeyer commented Oct 14, 2021

@riccardoferretti and I chatted in real-time and realized that I was getting hung up on trying to handle all the scenarios automatically, which was causing complexity.

The breakthrough was the reminder that we could ask the user to confirm our best guesses for note locations in the scenarios where we aren't certain.
Also, a realization that we can separate "how we resolve the settings" from "what we do with the resolved settings". I had combined the issues in my previous message.

As a result, I have drafted a design that uses a single foam.newNotesDirectory setting.


@riccardoferretti

foam.newNotesDirectory is a filepath, either relative or absolute that describes where new notes should be created.

Use cases

  • User wants all notes to be created in a global Zettelkasten directory
  • User wants notes to be created in a notes subdirectory of the current workspace

Scenarios

There are many many scenarios, which is part of the reason why it was so hard to reason about.

5 Editor states

State Meaning
file_within_workspace There is a workspace open, and the current editor is a file is within the workspace
file_outside_workspace There is a workspace open, but the current editor is a file outside of the workspace
just_file_open There is no workspace open, but there is a file open
just_workspace_open There is a workspace open, but no files are open
nothing_open There is neither a workspace nor a file open

9 resolved value states

When determining where to create a new note, we care about 2 resolved values: foam.newNotesDirectory and the template filepath metadata attribute

There are three possible states for each of them: non-existent, relative filepath, absolute filepath.

Together that makes 3 foam.newNotesDirectory states * 3 template filepath states * 5 editor states = 45 states

To make sure that we had a sensible answer to each of them, I wrote a little script to output all the states and manually verified that the results looked correct.

file_outside_workspace

After reviewing the 45 states, I found that my desired behaviour in the file_outside_workspace editor state was always the same as the behaviour while in the file_within_workspace editor state.

I ended up collapsing file_outside_workspace into file_within_workspace.

This was a style decision I made. It could have gone a different way. e.g., I could have decided to treated this state as similar to just_file_open instead.

I'd encourage you to have a think about what you'd expect Foam to do in this editor state. Perhaps you'll disagree and we'll talk more about it.

This reduced the number of states that I'm showing below to 3 * 3 * 4 = 27.

The 27 states

Below is the auto-generated list of scenarios, and the auto-generated answers to the question "In this state, where does the note get created?"
Take a look and make sure that these behaviours match your expectations. If they do, I'll redo the document in this PR and you can do another round of review.

Click to expand

Scenario 1: no foam.newNotesDirectory, no template filepath

(e.g., first time user)

  • A file open and a workspace open:
    • The new note is created under dirname(${currentEditor})
  • Only a file open (no workspace open):
    • Ask the user to confirm a location, defaulting to dirname(${currentEditor})
  • Only a workspace open (no files open):
    • Ask the user to confirm a location, defaulting to ${WORKSPACE_FOLDER}/
  • Nothing open:
    • Ask the user to confirm a location, defaulting to <home_dir>/

Scenario 2: absolute foam.newNotesDirectory, no template filepath

(e.g., user wants global notes directory)

  • A file open and a workspace open:
    • The new note is created under <foam.newNotesDirectory>/
  • Only a file open (no workspace open):
    • The new note is created under <foam.newNotesDirectory>/
  • Only a workspace open (no files open):
    • The new note is created under <foam.newNotesDirectory>/
  • Nothing open:
    • The new note is created under <foam.newNotesDirectory>/

Scenario 3: relative foam.newNotesDirectory, no template filepath

(e.g., user wants per-workspace notes subdirectory)

  • A file open and a workspace open:
    • The new note is created under ${WORKSPACE_FOLDER}/<foam.newNotesDirectory>/
  • Only a file open (no workspace open):
    • Ask the user to confirm a location, defaulting to dirname(${currentEditor})
  • Only a workspace open (no files open):
    • The new note is created under ${WORKSPACE_FOLDER}/<foam.newNotesDirectory>/
  • Nothing open:
    • Ask the user to confirm a location, defaulting to <home_dir>/<foam.newNotesDirectory>/

Scenario 4: relative foam.newNotesDirectory, relative template filepath

  • A file open and a workspace open:
    • The new note is created under ${WORKSPACE_FOLDER}/<foam.newNotesDirectory>/<template_filepath>
  • Only a file open (no workspace open):
    • Ask the user to confirm a location, defaulting to dirname(${currentEditor})
  • Only a workspace open (no files open):
    • The new note is created under ${WORKSPACE_FOLDER}/<foam.newNotesDirectory>/<template_filepath>
  • Nothing open:
    • Ask the user to confirm a location, defaulting to <home_dir>/<foam.newNotesDirectory>/<template_filepath>

Scenario 5: no foam.newNotesDirectory, absolute template filepath

  • A file open and a workspace open:
    • The new note is created under <template_filepath>
  • Only a file open (no workspace open):
    • The new note is created under <template_filepath>
  • Only a workspace open (no files open):
    • The new note is created under <template_filepath>
  • Nothing open:
    • The new note is created under <template_filepath>

Scenario 6: absolute foam.newNotesDirectory, absolute template filepath

  • A file open and a workspace open:
    • The new note is created under <template_filepath>
  • Only a file open (no workspace open):
    • The new note is created under <template_filepath>
  • Only a workspace open (no files open):
    • The new note is created under <template_filepath>
  • Nothing open:
    • The new note is created under <template_filepath>

Scenario 7: relative foam.newNotesDirectory, absolute template filepath

  • A file open and a workspace open:
    • The new note is created under <template_filepath>
  • Only a file open (no workspace open):
    • The new note is created under <template_filepath>
  • Only a workspace open (no files open):
    • The new note is created under <template_filepath>
  • Nothing open:
    • The new note is created under <template_filepath>

Scenario 8: absolute foam.newNotesDirectory, relative template filepath

  • A file open and a workspace open:
    • The new note is created under <foam.newNotesDirectory>/<template_filepath>
  • Only a file open (no workspace open):
    • The new note is created under <foam.newNotesDirectory>/<template_filepath>
  • Only a workspace open (no files open):
    • The new note is created under <foam.newNotesDirectory>/<template_filepath>
  • Nothing open:
    • The new note is created under <foam.newNotesDirectory>/<template_filepath>

Scenario 9: no foam.newNotesDirectory, relative template filepath

  • A file open and a workspace open:
    • The new note is created under ${WORKSPACE_FOLDER}/<template_filepath>
  • Only a file open (no workspace open):
    • Ask the user to confirm a location, defaulting to dirname(${currentEditor})
  • Only a workspace open (no files open):
    • The new note is created under ${WORKSPACE_FOLDER}/<template_filepath>
  • Nothing open:
    • Ask the user to confirm a location, defaulting to <home_dir>/<template_filepath>

The code

Yes, this means that I have an implementation (in Ruby) of the algorithm for determining where to create the note. Here it is:

Click to expand
def note_creation_location(editor_state, new_notes_directory, template_filepath)
  resolved_new_notes_directory = new_notes_directory.nil? ? "" : "<foam.newNotesDirectory>"
  resolved_template_filepath = template_filepath.nil? ? "" : "<template_filepath>"

  return "The new note is created under `#{resolved_template_filepath}`" if template_filepath == :absolute
  return "The new note is created under `#{File.join(resolved_new_notes_directory, resolved_template_filepath)}`" if new_notes_directory == :absolute
  return "The new note is created under `#{File.join("${WORKSPACE_FOLDER}", resolved_new_notes_directory, resolved_template_filepath)}`" if [:just_workspace_open, :file_within_workspace].include?(editor_state) && (new_notes_directory || template_filepath)

  if editor_state == :nothing_open
    base_dir = "<home_dir>"
  elsif editor_state == :just_workspace_open
    base_dir = "${WORKSPACE_FOLDER}"
  end

  if base_dir
    suggested_path = File.join(base_dir, resolved_new_notes_directory, resolved_template_filepath)
    return "Ask the user to confirm a location, defaulting to `#{suggested_path}`"
  end

  if editor_state == :just_file_open
    return "Ask the user to confirm a location, defaulting to `dirname(${currentEditor})`"
  end

  if editor_state == :file_within_workspace
    return "The new note is created under `dirname(${currentEditor})`"
  end

  nil
end

@riccardoferretti
Copy link
Collaborator

I ended up collapsing file_outside_workspace into file_within_workspace.

I agree with that decision.

A note on the editor states:

just_workspace_open: There is no workspace open, but no files are open

I assume there is a type and you meant: There is a workspace open, but no files are open right?


I had some thoughts on the algorithm, but while working on it I came up with the point below, so let me hold those thoughts for myself for now, to avoid confusion.


Yet there is something that doesn't feel right in the resolution complexity.

If the two of us are struggling so much with it, what are the odds a user would ever understand it and know how to progress up?

I wonder if the path here could be simplification via flattening the steps.

Based on the following premises:

  • a template has a filepath property
  • we provide a set of variables that can be used in the filepath property: WORKSPACE_ROOT, USER_HOME, CURRENT_FILE_DIR, BASE_DIR (which we can define as BASE_DIR = WORKSPACE_ROOT || CURRENT_FILE_DIR || USER_HOME). Also others e.g. NOTE_TITLE. These variables are populated according to the scenarios from above
  • filepath property must be resolved as absolute and has a sensible default, something like: ${BASE_DIR}/${NOTE_TITLE}.md --> /Users/riccardo/foam/My Note.md

When a new note is created, we look up the filepath of the associated template and resolve it.
We ask the user for confirmation of the path if:

  • one of the variables is not available when resolving the filepath property
  • the resulting filepath is not absolute

We populate the default with some best estimate, more or less based on your algorithm above, and we explain in the description of the input box that the reason why we are asking is because some variables could not be resolve (which helps the user learn how the system works)

I understand this is possible a drift from the original approach, but I feel the complexity of the resolution is replaced by something that is much more "transparent" from the user's POV.
Another possible drawback is backwards compatibility with existing templates.

What are your thoughts?

@movermeyer
Copy link
Collaborator Author

movermeyer commented Nov 8, 2021

@riccardoferretti

I assume there is a type and you meant: There is a workspace open, but no files are open right?

Yes. Fixed.


If the two of us are struggling so much with it, what are the odds a user would ever understand it and know how to progress up?

I actually think that the mental model that a user has to have is pretty simple, and it is incremental to match their "Foam maturity".
While I listed 27 cases in the previous comment, that's just for us who have to ensure that things are sensible in the edge cases.

Here is what a user's mental model is as they grow in their Foam maturity:

Stage 1: New user

New users have nothing configured. Their mental model is: "New notes are created beside my currently open note"

Stage 2: foam.newNotesDirectory

Then a user might want one of the two use cases for foam.newNotesDirectory:

  • Case A: All my notes should be created in a global notes directory.
  • Case B: All my notes should be created in a subdirectory of each workspace.

They look at the docs for foam.newNotesDirectory exactly once, set the value of foam.newNotesDirectory accordingly (An absolute path for Case A, relative for Case B, the docs show examples of each).
I do not think this is too big of a task, and critically, they will likely never need to think about it ever again. It's not really part of their mental model anymore; after setting it, Foam "just works" the way they want it to from then on.

Stage 3: Custom templates

If a user finds that they want to override the foam.newNotesDirectory behaviour in a particular workspace, they can using the filepath attribute in workspace templates.

Then a user might want one of the two use cases for filepath:

  • Case A: The notes for this workspace should be created in a global notes directory.
  • Case B: The notes for this workspace should be created in a subdirectory of this workspace.

They look up the documentation for the filepath attribute in the templates documentation, and set it accordingly (An absolute path for Case A, relative for Case B, the docs show examples of each).

(Aside: the documentation for foam.newNotesDirectory also contains a link to these docs as a hint: "For more fine-grain control, consider using the filepath [template attribute]")


I've realized that in Case B of Custom templates, the code I suggested in my last comment would append a relative filepath attribute to a relative foam.newNotesDirectory.
That was needlessly complicated. Instead, a relative filepath in a template will just be relative to the workspace directly.


I'll think about your new proposal, but I don't think that there's much complexity from a user's perspective in what I proposed.

@movermeyer
Copy link
Collaborator Author

@riccardoferretti

What are your thoughts?

I like it. I feel that our ideas are getting very close to each other.
Some comments (all discussion, no major criticisms)

Questioning backwards compatibility

filepath property must be resolved as absolute

We ask the user for confirmation of the path if:

  • [Case A] one of the variables is not available when resolving the filepath property
  • [Case B] the resulting filepath is not absolute

Another possible drawback is backwards compatibility with existing templates.

(Note: I added [Case A]/[CASE B] to the quote above so I could reference it)

I don't think we even need to break backwards compatibility here?

When does CASE B occur, without the original filepath not being explicitly relative?

  • Is there a case where filepath is still relative after using any of those variables?
    • I think using WORKSPACE_ROOT, USER_HOME, CURRENT_FILE_DIR, BASE_DIR in a filepath immediately makes it an absolute filepath.
      • And if one of them were not to resolve, we'd be in [CASE A]

Given [CASE A] will ask for confirmation, can't we continue to say that if the resulting filepath is not absolute, that it resolves relative to the workspace?
Put another way: If the filepath is relative, prepend WORKSPACE_ROOT to it, then do the resolution as described (hitting [CASE A] in the case where there is no open workspace)

i.e., I don't think backwards compatibility gets broken?

Resolution of the template file

  1. Check the open workspace (<workspace>/.foam/templates/new-note.md)
  2. Check the home directory (<home_dir>/.foam/templates/new-note.md)

If the template doesn't contain a filepath, then the notes continue to be created adjacent to the current open note (i.e., filepath defaults to ${CURRENT_FILE_DIR}/${FOAM_TITLE}):

filepath = <filepath_from_template> || ${CURRENT_FILE_DIR}/${FOAM_TITLE}
note_contents = <template_contents> || <default_contents>

Do we actually need BASE_DIR?

I wonder if we can get away without defining a BASE_DIR at all? I suspect that we can?
When would I want to use BASE_DIR over the other variable options?

Aside: Variable resolution

(This section is mostly for myself; reflecting on the variable resolution)

WORKSPACE_ROOT, USER_HOME, CURRENT_FILE_DIR, BASE_DIR

Aside: Some of these are already available in VSCode:

  • CURRENT_FILE_DIR is already available as TM_DIRECTORY
  • WORKSPACE_ROOT is already available as WORKSPACE_FOLDER

That said, since the resolution of these variables will have to happen before we send the snippet to VSCode, we'll have to have these resolvers in the Foam codebase.
At which point, we don't gain much by sticking with their names. Our resolvers will just be made to resolve either name (Foam's or VSCode's).

(I've been working on vendoring the VSCode snippet parsing code, so this is top of mind)

Differences between this proposal and my previous comment

No foam.newNotesDirectory

  • There is no foam.newNotesDirectory setting. The only way to customize the note paths is via filepath in templates.
    • OK. filepath is expressive enough.

I wonder if having foam.newNotesDirectory is a better onboarding experience though? See the maturity model I laid out in the previous comments.

That said, foam.newNotesDirectory can be added on after, if needed. YAGNI (though IDK how we'd ever hear that we need it. Users who feel overwhelmed by templates will just churn).

In my previous comment, it's effectively:

filepath = <filepath_from_template> || foam.newNotesDirectory || ${CURRENT_FILE_DIR}/${FOAM_TITLE}
contents = <template_contents> || <default_contents>

So adding foam.newNotesDirectory doesn't change the complexity all that much, just adds a conceptually simpler (?) way to specify filepath.

Summary

I think we're on the same page. I think this is simple enough, and robust enough to provide a good UX in all the edge cases. 👍

We can put off implementing foam.newNotesDirectory for now, but we have a plan for how it would fit within this solution.

Next steps:

  • Finalize the name of the variables needed, and write resolvers for them.
  • Rewrite the documentation to match
  • I need to finish my vendoring of the VSCode snippet parsing, which will eliminate so many edge case bugs

@riccardoferretti
Copy link
Collaborator

Backwards compatibility

Given [CASE A] will ask for confirmation, can't we continue to say that if the resulting filepath is not absolute, that it resolves relative to the workspace?
Put another way: If the filepath is relative, prepend WORKSPACE_ROOT to it, then do the resolution as described (hitting [CASE A] in the case where there is no open workspace)

The problem here is that we go back to the fallbacks... what if we are not in a workspace?

We could prompt the user for confirmation, but why not simply say "hey, this is the way you have to define a template filepath". It's also worth noting that by this point the user is fully onboarded in Foam, so it's a fair request for them to do a bit extra work ;)

I should also say that I am not super concerned about backwards compatibility here, because we wouldn't be changing behavior, we would simply tell the user to update the configuration so that we can univocally resolve the path - I consider it a very minor nuisance.

Basedir

I wonder if we can get away without defining a BASE_DIR at all? I suspect that we can?
When would I want to use BASE_DIR over the other variable options?

You are possibly right, and I am not very fussed around this.
The only case I was thinking of is having a user template, which can be used both within and outside of a workspace. But I think that's quite an edge case, happy to drop it.

New notes directory

I wonder if having foam.newNotesDirectory is a better onboarding experience though?

I strongly believe that the onboarding experience is better this way, I think a setting along these lines (but not quite what we originally thought of, see below) is quite important, as I have heard several comments/questions/issues raised around the location of new notes.

I have a feeling this is where 90% of users will settle on.

My thinking here:

  • this setting serves 2 use cases: workspace root, current dir
    • in case of just 'workspace root' and 'current dir', this is a simple enum for the new user
  • this setting will completely be ignored if a template defines a filepath property
  • the reason why I wouldn't even allow to specify a location in this property is that I think we can push that on the template "maturity level" for the advanced user

Code simplicity

There is a point I haven't explicitly made above: by using the variables approach we simplify, possibly quite considerably, our code (and hence its maintenance). Compare "just resolve the variables, is this absolute?" vs "if we are in workspace, do this; if we are with just a file open, do that; ...".

Same approach about the "new notes directory": we use that setting only if you don't have a filepath property defined, otherwise it's ignored.

To me it feels easier to explain, test, wrap one's head around, and maintain.

Summary

I think we are converging, I agree. There are a couple of side loops to wrap but the spiral is closing in :)
Let me know your thoughts above, this is very helpful and my thinking around this issue has developed a lot thanks to your work, I really appreciate it!

@chris-patenaude
Copy link

Just curious if theirs been any progress on this feature? I just started using Foam and am already in love, but I would really like to set a directory for newly created notes, lol. Thanks for all your hard work!

@riccardoferretti
Copy link
Collaborator

hi @chris-patenaude - yes I need to give this a higher priority, it's been a bit on the backburner.

Long story short, I am thinking of having a path that can be defined as a property or in the template which must resolve to an absolute path. If the resulting path is not absolute, a confirmation is asked to the user.

One way you can help achieve that is by helping getting 20 upvotes (if you have friends that use Foam please ask them too ;) ) on this issue with VS Code:
microsoft/vscode#155868
It will allow us to use some snippet variables in the resolution of the path

I am hoping to come back to this issue soon(-ish), its priority in any case is high

@chris-patenaude
Copy link

The call to arms has gone out and we've reached 20 up votes for microsoft/vscode#155868! 🥳

@riccardoferretti
Copy link
Collaborator

Closing for lack of activity and because in the meantime a few changes have mitigated the issue (although likely not completely resolved)

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.

3 participants