Add auto-start support - Githubissues

rejeep / prodigy.el

Manage external services from within Emacs

GNU General Public License v3.0

550 stars 39 forks source link

Add auto-start support #106

Closed seblemaguer closed 2 years ago

seblemaguer commented 6 years ago

I am currently working on adding the auto-start support.

[X] add the logic
[x] Add the documentation
[x] Add the test

This aims to solve #38

seblemaguer commented 6 years ago

I took the time (sorry for that, I had problem to understand ERT) but now it should be ready to be merged if you want :)

Fuco1 commented 5 years ago

Hey, looks pretty OK. Some of the formatting could use some love :)

I'm not sure how I feel about the explicit call to the auto start method.

I think the idea was to have it auto-start immeiately after prodigy-define-service is finished. However this might cause quite some trouble in case there are dependencies (which we don't have yet).

What should happen if I call the auto start function and then define another auto-start process during my session? We should at least explain the expected behaviour.

Fuco1 commented 5 years ago

Maybe we could use after-init-hook to call that function automatically?

seblemaguer commented 5 years ago

I have a look at this (about the formatting too :D) and I update the pull request

seblemaguer commented 5 years ago

Hello,

I had a little bit of time (lately) to rethink about it:

for the formatting, I actually don't really understand what is wrong. If you can tell me, that would be great as I can learn :)
for the method I added it in order to have a better control. The use after-init-hook seems a good idea for me.
for the testing, yes. I will see to add that.

seblemaguer commented 5 years ago

I refactor some stuff:

the indentation based on your comments (thanks by the way I was not away of the fact that you needed to load the helper file to get the indentation rules for the macro)
I also took into account your comment about the function. Now it doesn't depends if we activate the auto-start before or after. I didn't add the after-init-hook documentation but I can do that.
I rebase everything to be in 1 commit and to take into account the evolution of the master branch.

Fuco1 commented 5 years ago

@seblemaguer Thanks, looks good! I'll try to review this ASAP but please if I forget don't be afraid of being annoying and just ask for review! That helps me a lot to keep track of everything.

seblemaguer commented 5 years ago

@Fuco1 can we see to finish this pr? Else I am going to loose track of it for months again :/

rejeep commented 5 years ago

Please don't have inline comments, better describing them with a test. Comments get out of date.

seblemaguer commented 5 years ago

@rejeep I removed the comments

seblemaguer commented 5 years ago

Hello,

is there any news for this?

DamienCassou commented 2 years ago

I'm closing all old PRs. If you still care, please rebase your branch and reopen the PR.

seblemaguer commented 2 years ago

With context.el this pr became obsolete. Sorry I forgot to close it