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

Deprecate filetype_id #932

Merged
merged 3 commits into from Mar 6, 2016
Merged

Deprecate filetype_id #932

merged 3 commits into from Mar 6, 2016

Conversation

b4n
Copy link
Member

@b4n b4n commented Mar 4, 2016

Same as #931 but using a typedef. Also, there's no conflict.

@kugel- I fixed the GtkDoc header generation script not to break if a typedef type contains a reference. However, will the alias filetype_id confuse GIR or not?

@b4n b4n mentioned this pull request Mar 4, 2016
@kugel-
Copy link
Member

kugel- commented Mar 4, 2016

The alias was in the gir before also. No I dont think there's confusion.

@codebrainz
Copy link
Member

As mentioned in #931 comment, I remember some gint/guint where we could use the real type. Unfortunately I had a Git fail (forgot to pull after fetching @b4n's branches) so it clobbers some changes in this PR, but it shows all the places I could find after a relatively quick search:

https://gist.github.com/codebrainz/f024bf1128c948a1092e

Would probably require an API and ABI bump (if not done yet this release) since the sizeof enums isn't guaranteed to be the same as int/uint.

@techee
Copy link
Member

techee commented Mar 5, 2016

@b4n Looks good.

@b4n
Copy link
Member Author

b4n commented Mar 5, 2016

@codebrainz yeah there's quite a few places where we should use the typedef rather than gint/guint, I made a commit for that right after these, but didn't include it as I didn't necessarily check properly enough whether there might be side effects (I doubt it, but still). I left public API alone in my case, but yeah ultimately we should use the typedef in public functions too. There's however a few specific locations where we can't use the typedef because we use -1 as "not chosen" (i.e. in the file open dialog).
Anyway, I think I'd rather do this part after the release, esp. for the internal changes.

@kugel- it wasn't in the version right before this change, that's why I ask. But it's not documented so I guess it should get ignored just fine.

@kugel-
Copy link
Member

kugel- commented Mar 5, 2016

@b4n sorry, I meant it was not in the gir (because of lacking docs, due to an earlier PR of mine :-) . typo.

@codebrainz
Copy link
Member

@b4n sounds good. In next cycle, we could add something like GEANY_FILETYPE_INVALID=-1 GEANY_FILETYPE_AUTO=-1 to support the issue you mentioned.

@b4n
Copy link
Member Author

b4n commented Mar 6, 2016

@b4n sounds good. In next cycle, we could add something like GEANY_FILETYPE_INVALID=-1 GEANY_FILETYPE_AUTO=-1 to support the issue you mentioned.

Yeah. Though, does adding a negative member to an enum change ABI? :]
BTW, commit in question was 34aa652

@b4n b4n added this to the 1.27 milestone Mar 6, 2016
@b4n b4n self-assigned this Mar 6, 2016
@b4n b4n merged commit 9a85475 into geany:master Mar 6, 2016
b4n added a commit that referenced this pull request Mar 6, 2016
@codebrainz
Copy link
Member

Though, does adding a negative member to an enum change ABI?

I'm pretty sure any change to that enum besides maybe adding enumerators after GEANY_MAX_BUILT_IN_FILETYPES will break ABI. Even adding at the end might break ABI, I think, since the compiler could choose to use a different underlying type once it gets to a certain size. Anyway, it might be good to slip in next time ABI breaks for something else.

@kugel-
Copy link
Member

kugel- commented Mar 6, 2016

add the abi-todo tag somewhere, I've sneaked that into another place too, so we can grep for it :)

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 this pull request may close these issues.

None yet

4 participants