[WilliButz]

Previously configurable paths like tmpPath or uploadsPath were interpreted as relative paths to the base directory of CodiMD. Replacing path.join with path.resolve adds the possibility of making these configurable paths canonical and therefore independent of CodiMD's base directory.

A relative path in lib/models/revision.js caused an error to be thrown on startup when CodiMD was not startet from its base directory, this should now be fixed.

I added an optional override for the path of the config file by introducing the envrionment variable CMD_CONFIG_FILE to allow config.json to be outside of CodiMD's base directory.

I also updated the README so that users are informed about both the relative paths and the optional override.

[SISheogorath]

Some minor changes :)

[SISheogorath]

Can we use path.resolve() here as well? I see no point in not using resolve everywhere.

[SISheogorath]

Not sure if we explicitly need to point out this everywhere. From my perspective this can be expected. @ccoenen you as someone who has a good point of view on usability, what do you think about this?

[WilliButz]

I agree that this is very exhaustive, I intentionally added a note to each option so that no false interpretations happen when there is just a note for one of the options. Maybe a footnote in combination with a reference to it at each relative path makes more sense. :)

[SISheogorath]

Maybe we should split out the file options anyway. Would like to move them to an own document in the docs/ section. They are basically not needed by most users and the people who want to use them, should be able to find them in the docs/ section

[WilliButz]

@SISheogorath So I just changed my note from being a inline comment for each option to a reference of a footnote and rebased onto master.

While I do agree with you on moving parts of the README to a separate file in docs/, I think that it is out of scope for this PR. I'd to open an issue for restructuring the documentation if that is okay with you. (Edit: just found #334). I might even work on it myself in the future, but not in the next days :)

[SISheogorath]

We should use also use path.resolve here. And then put it like this:

const configFilePath = path.resolve(appRootPath, process.env.CMD_CONFIG_FILE || './config.json')

Seems more intuitive to me. Even when I'm still not happy with having two locations where we resolve environment variables now .-.

[WilliButz]

@SISheogorath a few days ago I realized that there is still something missing here but I haven't had much time until now. So basically the current state of this PR doesn't have any negative impact when the configurable paths aren't used, but it also doesn't work properly so far.

While the configurable tmpPath works as expected, the expressjs routing would need to be extended for the other options, so that the static files as well as the uploaded ones are served from the configured paths. I have working changes for this, but as I am not really familiar with express, I'd like to get your opinion on my proposed fix below before I add it to this PR :)

diff --git a/app.js b/app.js
index 24f0516..eef77f0 100644
--- a/app.js
+++ b/app.js
@@ -126,6 +126,15 @@ app.use(i18n.init)
 // routes without sessions
 // static files
 app.use('/', express.static(path.join(__dirname, '/public'), { maxAge: config.staticCacheTime }))
+app.use('/docs', express.static(path.resolve(__dirname, config.docsPath), { maxAge: config.staticCacheTime }))
+app.use('/uploads', express.static(path.resolve(__dirname, config.uploadsPath), { maxAge: config.staticCacheTime }))
+app.use('/default.md', express.static(path.resolve(__dirname, config.defaultNotePath), { maxAge: config.staticCacheTime }))
+app.use('/views/index.ejs', express.static(path.resolve(__dirname, config.indexPath), { maxAge: config.staticCacheTime }))
+app.use('/views/error.ejs', express.static(path.resolve(__dirname, config.errorPath), { maxAge: config.staticCacheTime }))
+app.use('/views/slide.ejs', express.static(path.resolve(__dirname, config.slidePath), { maxAge: config.staticCacheTime }))
+app.use('/views/codimd.ejs', express.static(path.resolve(__dirname, config.codimdPath), { maxAge: config.staticCacheTime }))
+app.use('/views/pretty.ejs', express.static(path.resolve(__dirname, config.prettyPath), { maxAge: config.staticCacheTime }))
+app.use('/views/js/lib/common/constant.ejs', express.static(path.resolve(__dirname, config.constantsPath), { maxAge: config.staticCacheTime }))
 
 // session
 app.use(session({

[SISheogorath]

Have a look at #947

@ccoenen worked on that, may want to check his work. You can probably also reach out to him in our Matrix community :)

[WilliButz]

@SISheogorath I updated this PR, kept the previous changes and fixed the file serving in
12cd747 and 825ee4e for when the configurable paths are different from their defaults. As mentioned on matrix, I included @ccoenen's commit and updated my changes to the README accordingly.

On my test instance everything works as expected™ now :)

[SISheogorath]

Thanks a lot for your work 👍 Hope to see more of it in future 😉