kovetskiy / mark

Sync your markdown files with Confluence pages.
https://samizdat.dev
Other
988 stars 147 forks source link

Feature/mark.d #355

Closed Skeeve closed 10 months ago

Skeeve commented 11 months ago

To remedy the concerns of #354, I've created this patch.

When an included file cannot be found, it will additionally be searched in the directory mark.d in the standard(!) config directory. So for Linux Systems in ~/.config/mark.d.

Unfortunately I currently see no way to take the --config parameter into account.

mrueg commented 11 months ago

What do you think of having an --include-dir command line option that is empty by default?

This would allow setting it to an arbitrary directory and being really explicit that this is the fallback if the include in the current directory fails.

Skeeve commented 11 months ago

Wouldn't be explicit, when defined in the settings file, wouldn't it? At least that's where I would define it.

Additionally I wouldn't know how to access the information, where the include-dir is because the cli settings are not accessible in templates.go.

mrueg commented 11 months ago

You can make them accessible if you add them to the ProcessIncludes() call in main.go and reuse them in LoadTemplate.

Depending on the usage of base string, it might make sense to refactor it to base string[] so it can handle multiple potential includes.

Skeeve commented 11 months ago

Do I understand correct? You suggest instead of passing one directory string in base, I could pass, a whole lot of directories, as base []string?

But be aware that, in this case, the paths in the --include-dir (or better --include-dirs? Or as environment variable MARK_LIBRARY_PATH?) have to be absolute paths.

mrueg commented 11 months ago

It would need some refactoring, but yes that would be the idea. Thinking again, maybe let's not reuse base but have another include variable that we pass to through the functions.

Why would those paths need to be absolute?

Skeeve commented 11 months ago

Why would those paths need to be absolute?

If they were not absolute, you would never know where they will point to. After all: The idea of this (kind of) library path is, to have a defined source for common include files.

BTW: Either we will have to pass that variable also to LoadTemplate, because there the ReadFile is done, or we should do the ReadFile as a first step in LoadTemplate and call it multiple times until the included file is found. But then we need to make sure that a found file, having an error, does not lead to trying to open the same file name in another location.

e.g. some "author.md" residing in the local directory and also somewhere in the include path. When the local one can be loaded but template processing yields an error, ProcessIncludes shall not try to use the file from the include directory instead.

mrueg commented 11 months ago

Why would those paths need to be absolute?

If they were not absolute, you would never know where they will point to. After all: The idea of this (kind of) library path is, to have a defined source for common include files. Ah yes you're correct here.

BTW: Either we will have to pass that variable also to LoadTemplate, because there the ReadFile is done, or we should do the ReadFile as a first step in LoadTemplate and call it multiple times until the included file is found. But then we need to make sure that a found file, having an error, does not lead to trying to open the same file name in another location.

e.g. some "author.md" residing in the local directory and also somewhere in the include path. When the local one can be loaded but template processing yields an error, ProcessIncludes shall not try to use the file from the include directory instead.

I agree, I would suggest only to fall back to the include directory in case the file does not exist in the path.

Skeeve commented 11 months ago

Okay. Would you agree that putting the includes as a variadic parameter at the end of ProcessIncludes and LoadTemplate is a good idea?

func ProcessIncludes(
    base string,
    contents []byte,
    templates *template.Template,
    includeDirs ...string,
) (*template.Template, []byte, bool, error) {
⋮
⋮
func LoadTemplate(
    base string,
    path string,
    left string,
    right string,
    templates *template.Template,
    includeDirs ...string,
) (*template.Template, error) {
⋮
⋮

That way both functions can be called like before, but in case of include directories being defined, LoadTemplate will handle them.

Skeeve commented 11 months ago

Push…

mrueg commented 10 months ago

Resolved as per https://github.com/kovetskiy/mark/pull/375