cknadler / vim-anywhere

Use Vim everywhere you've always wanted to
MIT License
3.67k stars 146 forks source link

Use mktemp to create temporary directory and files #85

Open lucaskanashiro opened 6 years ago

lucaskanashiro commented 6 years ago

Closes #81

cknadler commented 6 years ago

Hey Lucas,

Thanks for the PR. Do you know the compatibility of mktemp? Is it available by default on all modern versions of OSX and Linux?

lucaskanashiro commented 6 years ago

Hi,

I do not have an OSX system to test it, but after read this answer [1] I suppose that mktemp call passing the template should work.

[1] https://unix.stackexchange.com/questions/30091/fix-or-alternative-for-mktemp-in-os-x

lucaskanashiro commented 6 years ago

Someone with an OSX system could test this patch? Just to confirm my thoughts :)

mcpherrinm commented 6 years ago

mktemp -d works on Mac OS.

d1egoaz commented 6 years ago

FYI mktemp -p doesn't work on OSX

╰─○ mktemp -p
mktemp: illegal option -- p
usage: mktemp [-d] [-q] [-t prefix] [-u] template ...
       mktemp [-d] [-q] [-u] -t prefix

what about creating directly the file with:

TMPFILE="$(mktemp /tmp/vim-anywhere.XXXXX)"
function remove_tmp_file() {
    rm -rf $TMPFILE
}

## Removes all temp files on exit and sigint
trap "remove_tmp_file" EXIT SIGINT
lucaskanashiro commented 6 years ago

Thanks for the report @d1egoaz . I've tried to keep the same code structure and create temporary directory and file in a secure way. However, since the '-p' option is not available in OSX systems we can follow your approach and create the temp file directly. I'll update this PR.

lucaskanashiro commented 6 years ago

After think about this solution I do not know if @cknadler will agree with it, since we will not keep the history (one of the features described in README.md) anymore. What do you think @cknadler ?

simmel commented 6 years ago

Hope I'm not adding fuel to a fire here but this PR can be replaced by setting umask before creating the directory and files.

umask 077
mkdir -p $TMPFILE_DIR
touch $TMPFILE

Also the chmod o-r $TMPFILE # Make file only readable by you lines can be removed by this PR since that served this purpose but only for Linux.

Maybe we also want to clean up the permissions on the old files by doing chmod -R 600 $TMPFILE_DIR? 🤷‍♂️