Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GeanyLua] geany.activate not activating document #1229

Open
xiota opened this issue Mar 2, 2023 · 5 comments · May be fixed by #1234
Open

[GeanyLua] geany.activate not activating document #1229

xiota opened this issue Mar 2, 2023 · 5 comments · May be fixed by #1234

Comments

@xiota
Copy link
Contributor

xiota commented Mar 2, 2023

geany.activate() is supposed activate a document and return true. If it is unable, it returns false. However, it is not activating the document. Accepted arguments are a tab id (negative numbers), document id (positive numbers), or full document path. Test code attempting to call geany.activate() with the document id (test1) and path (test2) follows:

  test1 = geany.activate(geany.documents(geany.filename()))

  doc_id = geany.documents(geany.filename())
  test2 = geany.activate(doc_id)

  for key,val in pairs({ test1, test2 }) do
    if val then
      geany.status("true")
    else
      geany.status("false")
      geany.status(geany.documents(geany.filename()))
      geany.status(geany.filename())
    end
  end

Results in status window are:

false
1
/tmp/test
false
1
/tmp/test

For reference, an excerpt from the GeanyLua documentation:

geany.activate ( tab_id )

Activates the document specified by tab_id.

If tab_id is a number, it activates the document at that index. A positive number refers to the internal document array, and a negative number refers to the (absolute) GtkNotebook index.
Note that the indices begin at (1) and (-1) respectively.

If tab_id is a string, the function will try to activate the notebook tab whose filename matches that string.
( The full pathname is required! )

Returns true on success, or false if the tab could not be found.

@xiota
Copy link
Contributor Author

xiota commented Mar 4, 2023

Relevant code is in geanylua/glspi_doc.c. I don't see why it doesn't work.

/* Actvate and focus the specified document */
static gint glspi_activate(lua_State* L)
{
	gint idx=-1;
	if (lua_gettop(L)>0) {
		if (lua_isnumber(L,1)) {
			idx=(lua_tonumber(L,1));
			if (idx<0) { /* Negative number refers to (absolute) GtkNotebook index */
				idx=(0-idx)-1;
				if (idx>=gtk_notebook_get_n_pages(NOTEBOOK)) { idx=-1;}
			} else { /* A positive number refers to the geany->documents_array index */
				idx=doc_idx_to_tab_idx(idx-1);
			}

		} else {
			if (lua_isstring(L,1)) {
				idx=doc_idx_to_tab_idx(filename_to_doc_idx(lua_tostring(L, 1)));
			} else {
				if (!lua_isnil(L,1)) { return FAIL_STR_OR_NUM_ARG(1); }
			}
		}
	}
	if (idx>=0) {
		if (idx!=gtk_notebook_get_current_page(NOTEBOOK)) {
			gtk_notebook_set_current_page(NOTEBOOK, idx);
		}
	}
	lua_pushboolean(L, (idx>0));
	return 1;
}

@elextr
Copy link
Member

elextr commented Mar 4, 2023

I'm not sure the OPs code should do anything, correct me if I'm wrong, but AFAICT it gets the current document which is the one in the current notebook tab, then sets the notebook tab to that tab, but the code from above:

		if (idx!=gtk_notebook_get_current_page(NOTEBOOK)) {
			gtk_notebook_set_current_page(NOTEBOOK, idx);

makes it do nothing if the current tab == the tab requested == the tab of the current document == the document of the current tab so its guaranteed to do nothing.

Even if it did set the notebook tab GTK does not say it grabs focus, so the Geanylua doc that says:

function activate ( tab_id )                     -- Focus the specified editor tab.

is wrong anyway.

@xiota
Copy link
Contributor Author

xiota commented Mar 4, 2023

I'm not sure the OPs code should do anything...

In my Lua test code, what I'm interested in is the return value. Even if the function ultimately doesn't have to switch tabs, idx should still be set greater than 0 and return true. But no matter what I send, including numbers that should work, like 1, it returns false.

Even if it did set the notebook tab GTK does not say it grabs focus, so the Geanylua doc... is wrong anyway.

Okay. But the tab isn't changed. Even if focus isn't changed, changing the tab would be an improvement. (eg, if I'm on tab 3, call geany.activate(1), tabs don't change.) Somehow by the end of the function, idx <= 0, causing false to be returned. Maybe the problem is lua_gettop(L)>0 and idx is never changed from its initialized value?

Presumably, this function did work at some time in GeanyLua's past?

@elextr
Copy link
Member

elextr commented Mar 4, 2023

But the tab isn't changed ... (eg, if I'm on tab 3, call geany.activate(1), tabs don't change.)

Ok, thats pretty clearly a bug.

The OP seemed to keep using the current document so they would not have changed anything IIUC, so not sure what they meant to describe.

For this bug idx gets values assigned to it all over the place, is there a confusion between the axes, Lua tab index positive starting at 1, Lua doc index negative starting at -1, and notebook page number positive starting at 0 or negative for last, you know usual off by one thingy?

[Edit: maybe there should be two different local variables, lua_idx and notebook_page to avoid confusion.]

@xiota
Copy link
Contributor Author

xiota commented Mar 4, 2023

The OP seemed to keep using the current document so they would not have changed anything IIUC

Even though that code does not change the tab, the return value should still be true. According to the (erroneous) documentation, the only time it should return false is "if the tab could not be found". That code clearly demonstrates that the document id could be found (so the tab should exist). There is also a demo script info/list-open-files.lua that is supposed to change tabs using geany.activate(), but does not.

maybe there should be two different local variables, lua_idx and notebook_page to avoid confusion

I'll poke at glspi_activate() later to see if I can find where it's misbehaving. If I manage to find the problem, I'll try to rewrite it as you suggest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants