API enhancements

[jannis-baum]

Closes #103

[jannis-baum]

The scrolling is getting more complicated than I thought🥲 @tuurep can you help me with something?

1. What happens when you turn off the redirection to ~ and run viv some_file and then run it again on the current version (not this PR)? Does it open 2 tabs or does it open one and then the second time just focus the one that was already there?
2. Same as 1 but with redirection turned on
3. What happens when you use xdg-open with the absolute URL without tilde and tilde redirection turned off and do that twice? Again, 2 tabs or 1 tab+focus that tab?
4. Same as 3 but with redirection turned on and using the tilde URL

[tuurep]

Oh no :D

Noted, will be getting back to this another day.

[jannis-baum]

I don't need your answers to all those questions anymore, thanks anyways! I found a solution, I think this is ready for testing & review now :)

You can open a file at a scroll position with viv --scroll n where n is the corresponding line number. It should not open a second tab if you already have a viewer open, instead it should focus that tab and scroll to the position on it.

I guess I should still add --scroll/-s to the --help message. The version flag as well.

[tuurep]

viv --scroll / -s seems like a fantastic idea

So is the meaning of n, that the content of n is visible at the bottom of page? Like for example I do viv -s 10 README.md on the viv repo README, it doesn't scroll because line 10 is already visible from the top?

If I understand that correctly, then it seems to work

It should not open a second tab if you already have a viewer open, instead it should focus that tab and scroll to the position on it.

This however doesn't quite work, it does the scroll seemingly correctly on the existing tab, but then also opens a new tab with scroll at the top

[tuurep]

Actually it seems to only work inconsistently in the first place

When I keep closing tab and doing viv -s 94 README.md repeatedly, it appears if it's working every other time (half the time not scrolling at all)

[jannis-baum]

So is the meaning of n, that the content of n is visible at the bottom of page? Like for example I do viv -s 10 README.md on the viv repo README, it doesn't scroll because line 10 is already visible from the top?

It's the same thing that the POST request from Vim does: It scrolls the minimal possible amount so that the content corresponding to line n is in the viewport. I.e. when you newly open the page it will most likely end up at the bottom of the page.

This however doesn't quite work, it does the scroll seemingly correctly on the existing tab, but then also opens a new tab with scroll at the top

Shit ^^ what happens when you xdg-open a URL that is already open in some tab? Does it also open a new tab anyways? (not in particular for Vivify-URLs but just in general, also with e.g. GitHub or something)

When I keep closing tab and doing viv -s 94 README.md repeatedly, it appears if it's working every other time (half the time not scrolling at all)

Seems like you were very fast there! Up until now the clients only got cleaned up after not replying to a ping, i.e. if you closed the tab the server needed up to 2 seconds to know and be able to properly handle the scrolling. I just added something that also clears them when the socket is properly closed so it should always work almost immediately now :)

[jannis-baum]

Ah, and adding to the xdg-open topic: Is there some way you could make your browser open the existing tab from the command line based on only the URL? E.g. mybrowser --fancy-option-that-reuses-tabs http://some/url/that/is/already/open/in/a/tab

[tuurep]

Yeah it opens a new tab with xdg-open (or firefox)

So must be a firefox specific thing

As far as I can see with a quick search, it looks like there's no flag for that

Maybe achievable with some scripting but hard to say

[jannis-baum]

As far as I can see with a quick search, it looks like there's no flag for that

Hm, that's sad. I feel like it would be quite a significant UX-flaw not to reuse already open tabs. On macOS and Safari it works.

This also puts us into a dilemma of how to implement this because we can only either have the reusing work for systems where you can open already open tabs, or open a new one at the given scrolling position. Or of course we can also support both with an option, but then it'll be a super-hard-to-explain option that is easy to set incorrectly or not set at all.

One other approach I see is to not use open at all but try focussing the tab from the client-side JS. We can try and investigate if that's blocked by our browsers or if it works, but I imagine Safari for example will block it to be honest.

[jannis-baum]

Just for context, I found this

• Firefox setting that may be relevant: https://support.mozilla.org/en-US/questions/1226151
• Chrome extension that might do it: https://chromewebstore.google.com/detail/one-tab-only/cdkbmeoilbjdnbaonjnenjdafkgaeabo?hl=en

Other than that it would probably be possible with some scripting. I don't know how deep we want to get into this, especially since every combination of system/browser/window-manager might need its own script. What do you think about this @tuurep and what browser and window manager do you use?

[tuurep]

The firefox thing looks like it's not related to the URL being the same -case and I don't think it's usable for our case

Chrome extension looks very relevant however

Yeah it seems really difficult, even more so with the X/Wayland divide

I'm using Openbox (under X) window manager and Firefox

[jannis-baum]

Just saw this as well: https://github.com/balta2ar/brotab. That might actually cover a majority of use cases.

Anyways, if anything I would revert Vivify to give the option to specify an open-command so that people could (for example) put a script based on that tool there and have it reuse tabs. Is this something you would do if the option was there? Or is this topic not that important to you anyways?

Edit we could also make it implicitly fall through some options, e.g. first try with brotab and only if it's not available revert to xdg-open

[tuurep]

https://github.com/balta2ar/brotab

Huh, might be the coolest thing I've seen in a while

Well, I'm not used to tab reuse in any other context than this but it interests me

By far I'm most interested in the scrolling to the correct location on invoking :Vivify from nvim, but are these two cases related?

[jannis-baum]

https://github.com/balta2ar/brotab

Huh, might be the coolest thing I've seen in a while

Since Vivify I assume :p

Well, I'm not used to tab reuse in any other context than this but it interests me

By far I'm most interested in the scrolling to the correct location on invoking :Vivify from nvim, but are these two cases related?

Yes, 100% related. The way we would do it from Vim is by running viv --scroll.

[jannis-baum]

From giving brotab a quick read it seems that it should be quite doable to use that if it's installed and instead fall back to xdg-open. I.e. we could mention that people who want tab-reusing should install that and then ✨it just works✨ but it's not a needed dependency.

And then based on whether it uses (1) brotab or macOS' open vs. (2) xdg-open we decide if we can reuse an existing tab (1) or if we need to open a new tab at a scroll position (2).

This feature exploded completely haha, it seemed so easy and now it got so complicated

[tuurep]

Since Vivify I assume :p

Vivify is TheBest

The way we would do it from Vim is by running viv --scroll.

Yeah but does this mean that all cursor-page syncing is migrated to doing viv --scrolls?

Because for that case it's enough for me that :Vivify opens a new tab, no need to reuse a previous one (and not relevant - you know, on startup only)

[jannis-baum]

The way we would do it from Vim is by running viv --scroll.

Yeah but does this mean that all cursor-page syncing is migrated to doing viv --scrolls?

No. The POST request is for scrolling whatever is already open, viv --scroll is for opening and then scrolling.

Because for that case it's enough for me that :Vivify opens a new tab, no need to reuse a previous one (and not relevant)

Okay! If you think this is not that necessary then we can just not have it and the only problem we have is having to guess if our open command will be able to reuse tabs or not.

[tuurep]

Although yeah there was this issue:

When I open same file twice with -s 94, the first scrolls correctly, but the second scrolls the first tab, and then opens a new tab at the top

When my expectation would be: don't (re)scroll the existing one, only open the new tab and then scroll on that

[jannis-baum]

When I open same file twice with -s 94, the first scrolls correctly, but the second scrolls the first tab, and then opens a new tab at the top

When my expectation would be: don't (re)scroll the existing one, only open the new tab and then scroll on that

Yes, exactly, this is the issue with your browser not reusing the tab. If it would, then it would not open the second tab and instead focus the first one, which also scrolled. That's how it's intended

[jannis-baum]

I think I actually just figured out a different solution:

Instead of opening the URL to the requested file from the second vivify-server instance that will terminate and just serves as a CLI, we can send a POST request from there to the first (running server) instance and then have it open the URL itself and at the same time register something that will send the SCROLL message to the first (next) socket that connects at that path. Then it works no matter if the tab is reused or not and we don't have to do any ugly hacks.

EDIT this might be very hard to read haha, especially if you haven't seen the code yet🙈 But no worries, it's mostly for me to try it out next.

[tuurep]

Alright! I'll be back on this one, but I wanna go and do jannis-baum/vivify.vim#5 right now

[tuurep]

Hey, yep this works and overall fantastic

The only thing I can notice but I'm not sure of the expected behavior:

If you bulk-open the same file many times with different lines to scroll i.e.

$ viv README.md:50 README.md:100

only one scroll, other opens at the top. What do you think about that?

When files are different, both scroll. Also tested with 3 different and works.

The tests are very comfortable and I got them to run and pass first try by only glancing at what you documented about them.

Not sure what tests to add but noted for the future.

[jannis-baum]

If you bulk-open the same file many times with different lines to scroll i.e.

$ viv README.md:50 README.md:100

Yes, this is directly caused by what I wrote in the footnote here and in the comment

Vivify/src/routes/_open.ts

Lines 18 to 21 in 6237788

	// NOTE: if we ever want to properly consider having many clients to one
	// server (currently not smart because entire file system would be
	// exposed), we will have to protect this critical section between here and
	// the websocket of the client connecting in `src/sockets.ts`

We could "fix" this by adding something like a mutex or semaphore around the section I described in the comment but I am not sure how much sense this makes since live updates from an editor for example will always affect all clients (tabs) the same way, i.e. all will end up at the exact same state.

I have never done low level synchronization like this in Node/JavaScript so I'm not sure how much complexity it would introduce but I can definitely look into it. What do you think?

[tuurep]

Hmmm.. Do I read into it correctly that it could be a bit of a nightmare for a small detail? 😄

The use case of opening the same file at 2 places is sensible, but maybe not quite important. In fact could be one of our edgiest edge cases.

[tuurep]

I can approve this at this point but if there's something to consider we don't have to merge yet

[jannis-baum]

Hmmm.. Do I read into it correctly that it could be a bit of a nightmare for a small detail? 😄

Maybe haha. I can also just set myself a 10 minute timer and see how far I get and then decide :D Maybe I'll do that, we have to see how thread-safe objects in JS are and how/if Express/NodeJS even implements multithreading. If it's all just one thread it will be very easy.

[tuurep]

Ok sounds good!

[jannis-baum]

So… I have said this before but I think/hope this should finally be all done now ^^ The message queuing is a lot more robust this way I think, I just can't really test opening the same file multiple times at different scroll locations because of course Safari reuses the same tab for all requests ^^ So let me know if this works now :)

PS: If you want to look at what I changed to make this (hopefully) work now, you can just look at 808cd17, e.g. with git diff 808cd17^ 808cd17; after that I just refactored some stuff to make things easier to understand.

[tuurep]

Great!

Yes it works, for example

$ viv README.md:60 README.md:100 README.md:70

all as expected.

git diff 808cd17^ 808cd17

Thanks, I looked at it but hopefully won't have to go deep into it 😄 But sounds like good changes generally as well.

So, do you think it can be merged?

[jannis-baum]

Great, thanks! Yes, let's finally merge this