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

Enum rework #217

Merged
merged 3 commits into from Nov 22, 2018

Conversation

Projects
None yet
3 participants
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 20, 2018

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:enum-rework branch from dfbbbc5 to 146851f Nov 20, 2018

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 20, 2018

Looks good, but cairo_content_t seems seems still needed (or CairoContent renamed back)
In this way cairo-sys don't need glib dependency in Cargo.toml?

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:enum-rework branch from 146851f to fdcfa22 Nov 20, 2018

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Nov 20, 2018

I've put back all *_t aliases.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 20, 2018

Maybe leave enums in sys with values otherwise sys crate will be less usable?

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Nov 20, 2018

I prefer to move enums outside of it and just keep aliases. It's more consistent with other gtk-rs crates this way.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Nov 20, 2018

(And for failing crates, I'll have to update the corresponding parts by hand.)

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 20, 2018

Not all other gtk-rs crates have enums both in sys and normal?

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Nov 20, 2018

True but the missing __Unknown field really bothered me. What do you think of it @sdroege?

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 20, 2018

Usually we have #repr[C] enum in sys and rust enum with __Unknown in main crate.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 20, 2018

.. as you already created rust enums to main you can just restore enums in sys and IMHO it will be good.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Nov 20, 2018

Which crate has enums in sys? Didn't find one yet (looked into gtk and gdk only for the moment...)

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 21, 2018

@GuillaumeGomez Yes, I was wrong.
We use c_int type with constants instead enum in sys.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 21, 2018

.. but do it in cairo-rs-sys will be IMHO too bothersome.

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Nov 21, 2018

The -sys crate should contain type aliases for the enums, plus constants. Like we do for the other -sys crates. No enums or bitflags, which should instead be in the non-sys crate and map between the two (with an unknown variant).

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Nov 21, 2018

For reference, here's the script to generate the consts and replace the enums values:

import re


f = open('src/enums.rs', 'r')
lines = [line[:-1] for line in f]

all_variants = []

i = 0
while i < len(lines):
    if lines[i].startswith('impl Into<ffi::'):
        enum_name = lines[i].split(' for ')[1].split(' {')[0]
        ffi_enum_name = '_'.join(x.upper() for x in re.findall('[A-Z][^A-Z]*', enum_name))
        i += 3
        while i < len(lines):
            if lines[i].strip().startswith('}'):
                break
            if lines[i].strip().startswith('#'):
                i += 1
                continue
            variant_name = lines[i].split(enum_name + '::')[1].split(' => ')[0]
            if variant_name.startswith('__Unknown'):
                i += 1
                continue
            parts = re.findall('[A-Z][^A-Z]*', variant_name)
            if len(parts) == 0:
                parts = [lines[i].split(enum_name + '::')[1].split(' => ')[0]]
            name = ffi_enum_name + '_' + '_'.join([part.upper() for part in parts])
            value = lines[i].split(' => ')[1].split(',')[0]
            all_variants.append((name, value))
            new_line = lines[i].split(enum_name + '::')[0] + enum_name + '::' + ''.join(parts) + ' => ffi::' + name + ','
            lines[i] = new_line
            i += 1
    elif lines[i].startswith('impl From<ffi::'):
        enum_name = lines[i].split(' for ')[1].split(' {')[0]
        ffi_enum_name = '_'.join(x.upper() for x in re.findall('[A-Z][^A-Z]*', enum_name))
        if len(ffi_enum_name) == 0:
            ffi_enum_name = enum_name
        i += 3
        while i < len(lines):
            if lines[i].strip().startswith('}'):
                break
            if lines[i].strip().startswith('#'):
                i += 1
                continue
            variant_name = lines[i].split(enum_name + '::')[1].split(',')[0]
            if variant_name.startswith('__Unknown'):
                i += 1
                continue
            parts = re.findall('[A-Z][^A-Z]*', variant_name)
            if len(parts) == 0:
                parts = [lines[i].split(enum_name + '::')[1].split(',')[0]]
            name = ffi_enum_name + '_' + '_'.join([part.upper() for part in parts])
            value = lines[i].split(' => ')[1].split(',')[0]
            new_line = '            ffi::' + name + ' => ' + enum_name + '::' + ''.join(parts) + ','
            lines[i] = new_line
            i += 1
    i += 1

f = open('src/enums.rs', 'w')
f.write('\n'.join(lines) + '\n')

for (key, value) in all_variants:
    print('pub const ' + key + ': i32 = ' + value + ';')
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Nov 21, 2018

@GuillaumeGomez Thanks.
IMHO only last is remove [dependencies.glib] and [dependencies.gobject-sys] in sys.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Nov 21, 2018

I'll check if nothing needs them in cairo-sys and if not, I'll remove them.

@@ -10,12 +10,17 @@ use std::ops;
use ::paths::Path;
use ::font::{TextExtents, TextCluster, FontExtents, ScaledFont, FontOptions, FontFace, Glyph};
use ::matrices::{Matrix, MatrixTrait};
use ffi::enums::{
use ::enums::{

This comment has been minimized.

@sdroege

sdroege Nov 21, 2018

Member

Or directly use the re-exported enums from the crate root?

src/lib.rs Outdated
@@ -88,6 +87,8 @@ pub use enums::{
RegionOverlap,
SurfaceType,
};
#[cfg(any(feature = "v1_12", feature = "dox"))]
pub use enums::MeshCorner;

This comment has been minimized.

@sdroege

sdroege Nov 21, 2018

Member

Or just pub use enums::*;?

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Nov 21, 2018

IMHO only last is remove [dependencies.glib] and [dependencies.gobject-sys] in sys.

glib-sys is still needed for all the get_type() functions in the -sys crate at least. gobject-sys might be unneeded

@GuillaumeGomez GuillaumeGomez merged commit 592d8d5 into gtk-rs:master Nov 22, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:enum-rework branch Nov 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.