tbabej / vit

Placeholder to demostrate issue porting for VIT.
0 stars 0 forks source link

[VT-130] vit --watch option #129

Open tbabej opened 6 years ago

tbabej commented 6 years ago

David Patrick on 2017-02-01T18:35:44Z says:

While vit is running, if something changes with the data, vit has now way of knowing, and doesn't immediately reflect the change. With the --watch option, if any of the source files change, the screen gets reloaded. This will make vit more useful in a multi-wondow or multi-application scenario.

tbabej commented 6 years ago

Migrated metadata:

Created: 2017-02-01T18:35:44Z
Modified: 2017-04-24T13:57:36Z
tbabej commented 6 years ago

David Patrick on 2017-02-01T18:39:26Z says:

related (obviates?) to VT-93 and VT-47

tbabej commented 6 years ago

Eric Hymowitz on 2017-02-03T20:31:23Z says:

I'm not (by trade) a programmer. I'm not sure I understand your work flow or (for that matter) git. But I have a possible patch. It requires tons of testing. Right now, it's not an option, it's just "on". It should eventually be an option which sets the "$alarmtime" variable. But in the meantime, it's working for me, and maybe somebody else can test it and expand on it and improve it.

{code} diff --git a/env.pl b/env.pl index 10c5b60..f7deb66 100644

+our $alarmtime = 5; our $cli_args = ''; our $audit = 0; our (at)colors2pair; our $convergence = ''; our $current_command = 'unknown'; our $cursor_position = 'unknown'; +our $data_location = ''; our $default_command = 'next'; our $display_start_idx = 0; our $error_delay = 500000; @@ -121,6 +123,7 @@ require 'search.pl';

  1. ##################################################################
  2. main...

+alarm $alarmtime; &parse_args(); &parse_vitrc(); &init_shell_env(); {code}

tbabej commented 6 years ago

David Patrick on 2017-02-06T15:39:46Z says:

(at)Eric Good to see some movement here. If I understand your patch (dimly) rather than --watch (which I think would be closer to inotify, called when file changes) your fix is closer to #VT-47, reloading on a fixed interval.. which is ok too.

tbabej commented 6 years ago

Eric Hymowitz on 2017-02-06T17:37:04Z says:

It's sort of a cross between the two -- it checks after a fixed interval, but only reloads if required (the timestamp on the underlying file changed). Hopefully, the overhead by the subroutine (check the timestamp, do a subtraction, reset the alarm timer) is low enough that you could set the timer as low as a few seconds, but I haven't benchmarked it or anything.

inotify is (if i understand correctly) Linux-only, which probably meant

and I figured using SIGALRM was more portable.

It also looked like the inotify perl modules are event-driven -- the program has to stop what it's doing and read the inotify queue to see if anything important happened. If perl's Curses's getch() blocks, then you still won't have inotify doing updates without some kind of user activity happening first. (But I could be wrong. I didn't look that hard.)

My intent was that eventually, yes, you would/could use --watch=20 to set the number of seconds, and the default would be 0 (never do it) if --watch is not specified and (i don't know) 30? 5? if --watch is specified without an argument. I hadn't gotten that far yet.

tbabej commented 6 years ago

David Patrick on 2017-02-08T19:32:16Z says:

(at)Eric, well that sounds like the best of both worlds then :)

tbabej commented 6 years ago

Eric Hymowitz on 2017-02-17T15:35:15Z says:

I have an updated patch. vit uses the taskwarrior context, and context is stored in $HOME/.taskrc , so I'm now checking for updates on that file as well as pending.data .

If anybody is testing this, I'd love feedback. It's working fine for me, but I'm afraid to submit it if it's going to break something.

{code} diff --git a/env.pl b/env.pl index 10c5b60..cf5903d 100644

+our $alarmtime = 5; our $cli_args = ''; our $audit = 0; our (at)colors2pair; our $convergence = ''; our $current_command = 'unknown'; our $cursor_position = 'unknown'; +our $data_location = ''; our $default_command = 'next'; our $display_start_idx = 0; our $error_delay = 500000; @@ -122,6 +124,7 @@ require 'search.pl';

  1. ##################################################################
  2. main...

+alarm $alarmtime; &parse_args(); &parse_vitrc(); &init_shell_env(); {code}

tbabej commented 6 years ago

Eric Hymowitz on 2017-02-28T15:15:20Z says:

I found 2 bugs with external programs.

  1. If I open an external program (for example, "taskopen" with I think is part of "tasknotes") and the timer expires and the files have been updated, then vit will repaint the screen, and leave the terminal in a confused state.

  2. If changes are made by an external program that does not have the "reread" flag, then it's possible that vit won't see those changes.

I think I can fix these ... but if anybody is testing (as far as I know, nobody is), then maybe you'll notice these issues.

tbabej commented 6 years ago

Scott Kostyshak on 2017-02-28T19:16:00Z says:

Hi Eric,

I just wanted to let you know that I'm following this issue, and that your time won't go to waste. I am not testing, but I know that this issue is important to several VIT users so I will look at a patch when 1. you are satisfied with the patch and consider it ready for consideration, and 2. when I have time. It is a little easier for me to deal with patches if you make a pull request at https://github.com/scottkosty/vit but if that is not easy for you to do, don't worry about it.

Thanks for your work on this!

Scott

tbabej commented 6 years ago

Eric Hymowitz on 2017-04-24T13:57:36Z says:

Just a quick update. I haven't forgotten. I had a computer crash at home that cost me some time.

I'm pretty happy with the changes I've made on my home system, and I plan to generate and submit a patch soon.