/tmp never gets cleared out

[DiogoNeves]

If you remove a file in the project (e.g. in the finder), it still compiles when it shouldn't.

[DiogoNeves]

what do you think about cleaning it up on startup and maybe a menu option to do exactly the same?

Note: if implementing menu option, set all Ink files compilerVersionDirty to true

[joethephish]

Hrm, I'd rather that the user didn't have to worry about it. I'm thinking about a flow like:

• User creates A.ink which includes B.ink
• User plays, all fine.
• User renames B.ink to C.ink, but forgets to update the INCLUDE statement in A.ink
• User plays, all fine <-- wrong!

Probably what we need is an extra field in the compilerInstruction which knows how to kill old files. I'm worried that there may be edge cases though, including when the user modifies files outside of inky (which we don't deal with just yet, just thinking ahead).

Could probably just invalidate everything when the user does anything to do with file changes, to be robust. (i.e. effectively do a rm -rf in the specific /tmp/project directory, and as you say, set compilerVersionDirty to true on all.)

[DiogoNeves]

Actually, your future case of detecting file changes would also fix this I guess... maybe it's better to put the effort into file watching now. What do you think? I could have quick look at it 😄

I'll attack the problem in steps but it sounds like once file watching is setup, it's just setting the flags in the right file (and deleting if appropriate).

A case that may require asking the user for information is if the currently opened file is deleted elsewhere. For example, Atom leaves the file open but marks it as unsaved (once you save it doesn't need to ask for the file name though).

I need to work on a couple of other projects now but I'll try to come back to this later, is that ok with you?

[joethephish]

Sure, whenever you have time to spend on the project is absolutely fine with me!

I think we need to remember that there's always a difference between:

1. What's saved fully on disk
2. What's in memory
3. What's in /tmp

File watching may help with (1), but the kind of deletion I'm talking about could be done entirely in (2) (i.e. InkFile objects being added and removed from the InkProject), and will still affect (3).

On the subject of file watching/saving in general, yeah, we need to decide what our preferred behaviour is. Two interesting cases:

Case 1 - saved changes

1. User makes edits to a file, saves.
2. Source control (or other app) modifies file or deletes it.
3. Editor should reflect the change by showing modifications. In the case of deletion:
   • Sublime Text keeps the file open and marks it as unsaved. I don't like this, because it makes you feel like you're losing important changes, even though you reviewed them elsewhere.
   • I'd prefer that the editor simply removes files that have been deleted in the file system. If it's the only file that's open, then it should stay open, but without being marked as unsaved, so you can close it without a warning.

Case 2 - unsaved changes

1. User makes edits to a file, but doesn't save.
2. Source control (or other app) modifies a file or deletes it
3. Editor should ask user to resolve - would you like to reload the file(s) as they exist on disk?

[DiogoNeves]

Great points!! 👍 love the simple explanation.

I guess Case 1 should behave like Case 2 (ask) if it's the file you're currently looking at. Being the only opened file is a special case of that I guess.

What do you think?

There are two steps one this:
External changes to files ➡️ Changes to InkFile state ➡️ Changes to /tmp folder

What this means is, I came full circle again and think I can ignore the watching for now. I should work on making sure /tmp files reflect the Ink Files and Project state in memory. Then the watcher simply updates the in-memory state and that should subsequently change /tmp.

Makes it easier to test and everything because I can directly change the InkFile and see what happens.

This state sync should still happen in the current place, where it calls play, but I'll have to change my previous commits slightly to account for the difference between a file not being in the updatedFiles because it doesn't need processing or it when it was deleted. 😉

[joethephish]

Yup, exactly - we only need to consider InkFile -> /tmp for now. Sounds good!

(btw I removed a line you added that said "if there are no updated files, don't send the play message", since sometimes you want to re-play a story without any updated files)

[DiogoNeves]

cool, I wasn't sure about that one either but forgot to mention, sorry.

[luv2code]

I have a related issue. I have a linux system with multiple users, and whoever starts Inky first owns the /tmp/inky_compile folder and the other users cannot use Inky until that folder is deleted and recreated with under their account.

I think the ideal solution would be for Inky to respect the $TMPDIR env var ( it doesn't appear to) so I could set a per user tmp directory. But if the /tmp/ folder was cleaned up on exit, that would fix the problem too.

Let me know if I should open a new issue.

[joethephish]

Hmmm, I can't remember the reasoning but on Windows we use node.js's process.env.tmp but on Mac/Linux we use /tmp/inky_compile. See the line here:

inky/app/main-process/inklecate.js

Lines 28 to 31 in 1ce5846

	// TODO: Customise this for different projects
	// Is this the right temporary directory even on Mac? Seems like a bad practice
	// to keep files around in a "public" place that the user might wish to keep private.
	const tempInkPath = (process.platform == "darwin" || process.platform == "linux") ? "/tmp/inky_compile" : path.join(process.env.temp, "inky_compile");

I'm not a Linux person but I'm happy to take a (thoroughly tested!) pull request to change the behaviour.

[luv2code]

#223
I wasn't able to test on OSX; but I feel confident the behavior is the same as on Linux.

[SpoonMeiser]

I have a related issue. I have a linux system with multiple users, and whoever starts Inky first owns the /tmp/inky_compile folder and the other users cannot use Inky until that folder is deleted and recreated with under their account.

Heh, I just opened an issue for this: #369

[SpoonMeiser]

Looking into this, this looks like a platform independent way of creating a temporary directory: https://www.geeksforgeeks.org/node-js-fs-mkdtemp-method/ - in particular, is os.tmpdir not a platform independent way of determining the temp directory?