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

documents_array not exported #964

Closed
codebrainz opened this issue Mar 14, 2016 · 15 comments
Closed

documents_array not exported #964

codebrainz opened this issue Mar 14, 2016 · 15 comments

Comments

@codebrainz
Copy link
Member

My Code-Format plugin uses the documents_array symbol, but this symbol is not exported to plugins anymore. Got an undefined symbol error for this in the Debug Messages. Adding GEANY_API_SYMBOL macro to documents_array in document.c fixes the issue.

This was always broken on Windows, and since the linkage-cleanup changes, on Linux too.

@elextr
Copy link
Member

elextr commented Mar 14, 2016

Can you not use document_find_by_id() as is recommended by the API docs?

But, as its documented to be in the API it looks simply like its a bug.

@codebrainz
Copy link
Member Author

@elextr no, I need to loop over each document.

@elextr
Copy link
Member

elextr commented Mar 14, 2016

@codebrainz you could use the hated foreach macro then.

There is an argument to be made that given the semantics of the pointers in documents_array could be dangerous, since they can point to differing documents during their lifetime, it might be better to remove the array from the API instead, and leave the recommended document_find_by_id() as the only access.

@codebrainz
Copy link
Member Author

document_find_by_id() doesn't allow you to iterate over the documents.

@elextr
Copy link
Member

elextr commented Mar 14, 2016

I said the foreach macro

@codebrainz
Copy link
Member Author

The foreach_document() macro needs to access the documents array, if you remove access to the documents array, that macro can't work.

@elextr
Copy link
Member

elextr commented Mar 14, 2016

IIUC it uses a pointer hidden in another structure, not the direct access to the symbol.

@codebrainz
Copy link
Member Author

It accesses geany_data->documents_array, yes, but that still gives access to the document pointers (ex. via the documents macro). It would be no safer with respect to the lifetime issues.

@elextr
Copy link
Member

elextr commented Mar 14, 2016

Oh, I misread it, it returns an index to the array, I read it as returning an ID, ok, we probably need a foreach_document_id macro.

@codebrainz
Copy link
Member Author

I think we should just remove the global variables like this which are already accessible through geany_data from the plugin API (ie. remove the doc-comments, or move them to the geany_data member docs). They were already broken on Windows since the beginning and for a few versions on Linux and friends and it hasn't caused a huge outcry, so it's probably safe to do.

@b4n
Copy link
Member

b4n commented Mar 14, 2016

Can't you use GeanyData::documents_array instead of the global?

@b4n
Copy link
Member

b4n commented Mar 14, 2016

Okay, looks like we broke API at some point, but is it wise to re-add that when it's accessible through GeanyData?

@codebrainz
Copy link
Member Author

No I don't think it's wise. I think we should just remove it('s docs) from the plugin API. It's cleaner to access it through geany_data, and the macro madness already uses that pointer anyway.

codebrainz referenced this issue Mar 14, 2016
…ration

Major changes are:

- Some types were accidentally documented, even though they couldn't be
accessed by any exported API functions. Those are removed (especially
from encodings.h).

- Some types were not documented where they should. Documentation is
added for them. Members are not necessarily documented separately if names
are self-explanatory.

- @A XXX refers to parameters of the function, it's inappropriate for
highlighting NULL (change to @c)

- As per consensus, build_info is removed from GeanyData (replaced by
pointer to avoid ABI break; added grep-able abi-todo tag so it doesn't get
forgotten)
codebrainz added a commit to codebrainz/geany that referenced this issue Mar 14, 2016
The global was never accessible to plugins on Windows and hasn't been
accessible to plugins on Linux and others since the linkage-cleanup
changes.

Move documentation from the global variable to the GeanyData member
of the same name.

Closes geany#964
@kugel-
Copy link
Member

kugel- commented Mar 15, 2016

It accesses geany_data->documents_array, yes, but that still gives access to the document pointers (ex. via the documents macro). It would be no safer with respect to the lifetime issues.

what life time issues? Since we are single threaded we can safely assume that documents don't go away during a loop

@codebrainz
Copy link
Member Author

what life time issues?

If a plugin held on to a GeanyDocument pointer and a document was closed and one was opened, the GeanyDocument pointer might still point to a document but a different document. It's not an issue during loops, and exists for any function that gives access to a GeanyDocument, I'm just telling the issue @elextr is talking about.

@b4n b4n added this to the 1.28 milestone Jun 23, 2016
@b4n b4n closed this as completed in #966 Jun 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants