Skip to content

fpp: add a --keep-open flag#61

Closed
davvid wants to merge 1 commit into
facebook:masterfrom
davvid:keep-open
Closed

fpp: add a --keep-open flag#61
davvid wants to merge 1 commit into
facebook:masterfrom
davvid:keep-open

Conversation

@davvid
Copy link
Copy Markdown

@davvid davvid commented May 8, 2015

Add a flag that allows you to keep fpp running after running a command
or editing a file.

Run the current command but do not exit fpp when the -k|--keep-open flag
is specified on the command-line.

Add a flag that allows you to keep fpp running after running a command
or editing a file.

Run the current command but do not exit fpp when the -k|--keep-open flag
is specified on the command-line.
@lastquestion
Copy link
Copy Markdown
Contributor

👍 this is really nice and I have a parallel PR that's worse then your implementation

@pcottle
Copy link
Copy Markdown
Contributor

pcottle commented May 9, 2015

Thanks for the PR @davvid ! Curious about the workflow here -- the only tricky thing is now the execution of the shell script is split between python and the bash script (since now we have the os.system('sh ~/.fbPager.sh; rm -f ~/.fbPager.sh') so it complicates things like #59 where we need to decorate the bash script with a few headers

Also does this work for editing files? We would need to connect to dev/tty in that case right?

I almost wonder if we could implement this instead as pure bash, something like

$PYTHONCMD "$BASEDIR/src/processInput.py"
function runChooser {
  exec 0<&-
  chose.py // etc
  sh ~/.fbPager.sh < /dev/tty
}

if $1 == '--keep-open'; then
  while [true]:
    runProgram()
else
  runProgram()
fi

since we save the pickled selection and the files, this should give almost the exact same behavior without introducing the code complexity. what do you think?

@davvid
Copy link
Copy Markdown
Author

davvid commented May 9, 2015

I did test it with editing files (that was my use case: vim and gvim), so that did work, but as you said, it made the code a bit complex.

If no one beats me to it then I'll try to rework it with your suggestions in mind. Thanks for the review

@pcottle
Copy link
Copy Markdown
Contributor

pcottle commented May 10, 2015

If no one beats me to it then I'll try to rework it with your suggestions in mind. Thanks for the review

no problem @davvid, thanks for the interest in the project! After seeing how many changes have (recently) gone into the python stack and how many are planned (like moving the location of the pickled files), it'd be nice to keep that as simple as possible.

@pcottle
Copy link
Copy Markdown
Contributor

pcottle commented May 10, 2015

(lemme know if you cant get around to the refactor and I can take a stab and close out this PR)

@davvid
Copy link
Copy Markdown
Author

davvid commented May 10, 2015

thanks y'all. please feel free to take a stab at it -- it'll probably be
about a week or two before I'll have a chance to take a look.
On May 10, 2015 11:10 AM, "Peter Cottle" notifications@github.com wrote:

(lemme know if you cant get around to the refactor and I can take a stab
and close out this PR)


Reply to this email directly or view it on GitHub
#61 (comment).

@pcottle
Copy link
Copy Markdown
Contributor

pcottle commented May 11, 2015

yeah sure lemme take a stab at this

@pcottle
Copy link
Copy Markdown
Contributor

pcottle commented May 11, 2015

Boom!

[pcottle:~/Dropbox (Facebook)/wip/PathPicker:master]$ ./fpp -ko
Using old result...
nothing to do!
Using old result...
nothing to do!
Using old result...
nothing to do!
Using old result...
executing command:
cat '/Users/pcottle/Dropbox (Facebook)/wip/PathPicker/scripts/buildAndTest.sh' '/Users/pcottle/Dropbox (Facebook)/wip/PathPicker/src/format.py' | wc -l
     199
Using old result...
2 files to edit
Using old result...
nothing to do!
Using old result...

through the magic of stack overflow and ridiculous bash programming we have this down 🎉 🎉

@pcottle pcottle closed this in 72575f6 May 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants