~martanne/devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 2

[RFC vis] lua: allow Lua to change displayed files

Details
Message ID
<20240504133105.2289492-1-florian.fischer@muhq.space>
DKIM signature
permerror
Download raw message
This patch allows Lua to change the file displayed in a window
by simply writing the file's name to the win.file member.

Why is this useful?

Mr Alin and I were busy writing a usable file lock plugin [1] in Lua the last
couple days. It is somewhat finished.
How ever one use case is not well supported.

If you open a file and then try to edit a locked file using the edit command,
there is currently no good way to abort the edit attempt and switch back to the
previously opened file with the current Lua API.

We tried all sorts of things: hooking various events, delaying the change to
subsequent UI_DRAW events, ... .

This patch solves this use case because Lua now can simply reset the opened
file in the new window created by cmd_edit during the WIN_OPEN event [2].


What do you think?

Are there any concerns about changing a window's file from Lua?
Do you know of any corner case I am missing?

Best,
Flo

[1]: https://gitlab.com/muhq/vis-lockfiles
[2]: https://gitlab.com/muhq/vis-lockfiles/-/merge_requests/6

[PATCH] allow lua to change a windows file

Details
Message ID
<20240504133105.2289492-2-florian.fischer@muhq.space>
In-Reply-To
<20240504133105.2289492-1-florian.fischer@muhq.space> (view parent)
DKIM signature
permerror
Download raw message
Patch: +16 -0
---
 vis-lua.c |  3 +++
 vis.c     | 11 +++++++++++
 vis.h     |  2 ++
 3 files changed, 16 insertions(+)

diff --git a/vis-lua.c b/vis-lua.c
index 4d783c5d..cb328b66 100644
--- a/vis-lua.c
+++ b/vis-lua.c
@@ -2058,6 +2058,9 @@ static int window_newindex(lua_State *L) {
			}
			lua_pop(L, 1);
			return ret;
		} else if (strcmp(key, "file") == 0 && lua_isstring(L, 3)) {
			const char* filename = strdup(lua_tostring(L, 3));
			vis_window_change_file(win, filename);
		}
	}

diff --git a/vis.c b/vis.c
index 0c05558a..7f005057 100644
--- a/vis.c
+++ b/vis.c
@@ -505,6 +505,17 @@ bool vis_window_reload(Win *win) {
	return true;
}

bool vis_window_change_file(Win *win, const char* filename) {
	File *file = file_new(win->vis, filename);
	if (!file)
		return false;
	file->refcount++;
	if (win->file)
		file_free(win->vis, win->file);
	win->file = file;
	view_reload(win->view, file->text);
}

bool vis_window_split(Win *original) {
	vis_doupdates(original->vis, false);
	Win *win = window_new_file(original->vis, original->file, UI_OPTION_STATUSBAR);
diff --git a/vis.h b/vis.h
index 312e87c4..b7192c6f 100644
--- a/vis.h
+++ b/vis.h
@@ -212,6 +212,8 @@ bool vis_window_new(Vis*, const char *filename);
bool vis_window_new_fd(Vis*, int fd);
/** Reload the file currently displayed in the window from disk. */
bool vis_window_reload(Win*);
/** Change the file currently displayed in the window. */
bool vis_window_change_file(Win*, const char *filename);
/** Check whether closing the window would loose unsaved changes. */
bool vis_window_closable(Win*);
/** Close window, redraw user interface. */
-- 
2.45.0

Re: [PATCH] allow lua to change a windows file

Details
Message ID
<3EPT1G1PDGOTZ.2EZUFYXV51G2F@homearch.localdomain>
In-Reply-To
<20240504133105.2289492-2-florian.fischer@muhq.space> (view parent)
DKIM signature
missing
Download raw message
Heyhey!

Florian Fischer <florian.fischer@muhq.space> wrote:
> ---
>  vis-lua.c |  3 +++
>  vis.c     | 11 +++++++++++
>  vis.h     |  2 ++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/vis-lua.c b/vis-lua.c
> index 4d783c5d..cb328b66 100644
> --- a/vis-lua.c
> +++ b/vis-lua.c
> @@ -2058,6 +2058,9 @@ static int window_newindex(lua_State *L) {
>  			}
>  			lua_pop(L, 1);
>  			return ret;
> +		} else if (strcmp(key, "file") == 0 && lua_isstring(L, 3)) {
> +			const char* filename = strdup(lua_tostring(L, 3));

Theoretically strdup can return NULL so we should probably check for that.

Also, I think this currently leaks memory. We should free the memory
that strdup allocates.

Cheers,
Silvan


> +			vis_window_change_file(win, filename);
>  		}
>  	}
>  
> diff --git a/vis.c b/vis.c
> index 0c05558a..7f005057 100644
> --- a/vis.c
> +++ b/vis.c
> @@ -505,6 +505,17 @@ bool vis_window_reload(Win *win) {
>  	return true;
>  }
>  
> +bool vis_window_change_file(Win *win, const char* filename) {
> +	File *file = file_new(win->vis, filename);
> +	if (!file)
> +		return false;
> +	file->refcount++;
> +	if (win->file)
> +		file_free(win->vis, win->file);
> +	win->file = file;
> +	view_reload(win->view, file->text);
> +}
> +
>  bool vis_window_split(Win *original) {
>  	vis_doupdates(original->vis, false);
>  	Win *win = window_new_file(original->vis, original->file, UI_OPTION_STATUSBAR);
> diff --git a/vis.h b/vis.h
> index 312e87c4..b7192c6f 100644
> --- a/vis.h
> +++ b/vis.h
> @@ -212,6 +212,8 @@ bool vis_window_new(Vis*, const char *filename);
>  bool vis_window_new_fd(Vis*, int fd);
>  /** Reload the file currently displayed in the window from disk. */
>  bool vis_window_reload(Win*);
> +/** Change the file currently displayed in the window. */
> +bool vis_window_change_file(Win*, const char *filename);
>  /** Check whether closing the window would loose unsaved changes. */
>  bool vis_window_closable(Win*);
>  /** Close window, redraw user interface. */

Re: [PATCH] allow lua to change a windows file

Details
Message ID
<xs7dqg4idbdpqiukblsn6u5wwwwa524u5xfbehzlfoifp7m6z5@aovb2egyhb7a>
In-Reply-To
<3EPT1G1PDGOTZ.2EZUFYXV51G2F@homearch.localdomain> (view parent)
DKIM signature
permerror
Download raw message
> Florian Fischer <florian.fischer@muhq.space> wrote:
> > ---
> >  vis-lua.c |  3 +++
> >  vis.c     | 11 +++++++++++
> >  vis.h     |  2 ++
> >  3 files changed, 16 insertions(+)
> > 
> > diff --git a/vis-lua.c b/vis-lua.c
> > index 4d783c5d..cb328b66 100644
> > --- a/vis-lua.c
> > +++ b/vis-lua.c
> > @@ -2058,6 +2058,9 @@ static int window_newindex(lua_State *L) {
> >  			}
> >  			lua_pop(L, 1);
> >  			return ret;
> > +		} else if (strcmp(key, "file") == 0 && lua_isstring(L, 3)) {
> > +			const char* filename = strdup(lua_tostring(L, 3));
> 
> Theoretically strdup can return NULL so we should probably check for that.
> 
> Also, I think this currently leaks memory. We should free the memory
> that strdup allocates.

You are totally right.

The strdup is a leftover from a previous iteration of this change.
In this patch it is completely useless, because file_new already creates a copy
of the passed file name.

I will remove the strdup call in v2 in the future.

Flo

Re: [PATCH] allow lua to change a windows file

Details
Message ID
<2XZDHOJ7NC5AG.3VC8G33JLPUN4@homearch.localdomain>
In-Reply-To
<xs7dqg4idbdpqiukblsn6u5wwwwa524u5xfbehzlfoifp7m6z5@aovb2egyhb7a> (view parent)
DKIM signature
missing
Download raw message
Florian Fischer <florian.fischer@muhq.space> wrote:
> > Florian Fischer <florian.fischer@muhq.space> wrote:
> > > ---
> > >  vis-lua.c |  3 +++
> > >  vis.c     | 11 +++++++++++
> > >  vis.h     |  2 ++
> > >  3 files changed, 16 insertions(+)
> > > 
> > > diff --git a/vis-lua.c b/vis-lua.c
> > > index 4d783c5d..cb328b66 100644
> > > --- a/vis-lua.c
> > > +++ b/vis-lua.c
> > > @@ -2058,6 +2058,9 @@ static int window_newindex(lua_State *L) {
> > >  			}
> > >  			lua_pop(L, 1);
> > >  			return ret;
> > > +		} else if (strcmp(key, "file") == 0 && lua_isstring(L, 3)) {
> > > +			const char* filename = strdup(lua_tostring(L, 3));
> > 
> > Theoretically strdup can return NULL so we should probably check for that.
> > 
> > Also, I think this currently leaks memory. We should free the memory
> > that strdup allocates.
> 
> You are totally right.
> 
> The strdup is a leftover from a previous iteration of this change.
> In this patch it is completely useless, because file_new already creates a copy
> of the passed file name.

Ah, yes, you are correct!


> I will remove the strdup call in v2 in the future.

Sounds good to me, thanks!

Cheers,
Silvan
Reply to thread Export thread (mbox)