Page MenuHomePhabricator

Extract edje_entry into its own object that inherits from evas_canvas_text
Closed, ResolvedPublic

Description

The idea of this task is to split out edje_entry.c to its own file. It's OK to cause duplication now (so just copy the file and trim it). This task is essentially about creating Efl2.Text.Raw_Editable, though because Efl2.Canvas.Text is not yet ready, it's better to just create this object as if it inherited from Efl.Canvas.Text (note it's not Efl2!), so the main part of the work, the extraction from edje, can be done asap.

It's really just about making a standalone object that has no theme and can accept user input, IMF, and etc. So don't remove any functionality, but also don't be fancy and add anything, it's really just about extracting it from edje.

Please let me know if you need any clarifications.

tasn created this task.Aug 30 2019, 2:05 AM

I have these comments:
1- efl.ui or efl.canvas (not Efl2.Text) are the namespace for widget/canvas components.
2- I think we still need to think for good name for Raw_Editable
3- Can we create themeable object (efl.ui.text) which depend on Raw_Editable which has no theme ?

  1. What is the difference between existing "Efl.Ui.Internal.Text.Interactive" and the new one you want us to implement ?
  1. I'm ok to create new class with this purpose - but , for that, we need to think of definition of "Raw_Editable" first. So, How about discussing "the definition of Raw_Editable" here. If it will be the same with old edje_entry, then we don't need to - but some properties (such as password property) can be checked again whether it should be in this class or efl.text.canvas.

@tasn
Could you share your idea on this ?

tasn added a comment.Sep 2 2019, 12:59 AM

@woohyun:

My idea is already fully shared!
https://git.enlightenment.org/core/efl.git/tree/src/lib/elementary/efl2_text_raw_editable.eo?h=devs/tasn/ifaces

  1. Very different. The internal one that exists now is just a small implementation of some stuff, this one is a replacement for edje_entry (so handles all of the input and selection and etc) + password_mode + etc. This task is however not about creating this object fully yet, just splitting out the entry functionality from edje so we don't have to rely on edje anymore.
  2. Happy to discuss here, but whoever undertakes this task doesn't need to go that far. It's really just about taking what's already there in edje_entry.c and implementing it in another object.
tasn added a comment.Sep 2 2019, 1:01 AM

@ali.alzyod ,

  1. This is not a canvas component, and anyhow, I'm happy with changing this name completely. This task is orthogonal to that, because we can just sed the file to change the name afterwards.
  2. I agree, see previous point.
  3. Yes, that's exactly what's going to happen. This is just a replacement to edje_entry, so just the internal keyboard/mouse interaction implementation.
ali.alzyod added a comment.EditedSep 2 2019, 1:38 AM

@tasn

If we replace all edje_entry functionality to be in Efl2.Text.Raw_Editable, it will make Raw_Editable feature rich object. (it is ok when used with efl.ui.text).

But for example label should be lightweight widget but it is inherited from efl.ui.text and it does not (I think) use most of its features like select or insert, so it will not be lightwight in compare with efl_ui_text

So what do you think about :
efl.ui.text (does not have interactive features, nor selection features)
efl.ui.edittext (extend efl.ui.text and have selection and insertion features). contain all functinality from current efl.ui.text and efl.text.raw_edit

So for Viewing only purposes user uses efl.ui.text, and for interactive purposes user users efl.ui.edittext.

tasn added a comment.Sep 2 2019, 8:55 AM

This is orthogonal to this issue. This needs to be done regardless if you change it as you suggested or not. Could you please post the same comment on the text proposal ticket? T8151

Thanks!

This is orthogonal to this issue. This needs to be done regardless if you change it as you suggested or not.

You are right, I will check with @woohyun to start working on this

ali.alzyod added a comment.EditedSep 3 2019, 1:03 AM

I think we can reuse the code in efl.ui.internal.interactive, and move the remaining from edje_entry into Efl.Text.Raw_Editable

Do you agree @tasn ?

tasn added a comment.Sep 3 2019, 1:11 AM

I think the first step would be to anyway move edje_entry to its own object, after that we can figure out if we want to merge them together. We probably do want that in the long run, so you are right, but also good to have the isolated edje_entry split out.

I think the first step would be to anyway move edje_entry to its own objec

Do you mean EO object ?

tasn added a comment.Sep 3 2019, 1:33 AM

Yeah, so essentially edje_entry.c should become raw_editable, the eo object that's separated from edje (and resides in src/lib/elemenetary). At the moment it should inherit from efl_canvas_text.eo because efl2_canvas_text.eo is not yet implemented.

ali.alzyod added a comment.EditedSep 3 2019, 6:49 AM

At this stage
For example We will replace events like _edje_emit("selection,cleared") with efl_event_callback_call(EFL2_TEXT_RAW_EDITABLE_SELECTION_CLEARED), right ?

tasn added a comment.Sep 3 2019, 7:29 AM

Yeah, for instance.

@ali.alzyod - Will you work in "devs/tasn/ifaces" branch or make your own ?

I also want to check the modifications together.

ali.alzyod added a comment.EditedSep 4 2019, 12:50 AM

@tasn
Regard this task
Should we make our start point :

1- efl_ui_internal_interactive_text:

a- replace class name/data with efl_text_raw_edit
b- change events to use raw_editable events.
c- port missing edje_entry code to efl_text_raw_editable depending events/function/properties of current efl_text_raw_editable.eo file.

Or
2- Create new file from edje_entry

a- Generate efl_text_raw_edit headers/source files
b- Port evertthing from edje_entry to efl_text_raw_edit source files

@tasn @ali.alzyod

If my understanding is correct -
"efl_ui_internal_text_interactive" extends "efl_canvas_text",
and has covered some features of edje_entry (except the features of EDJE)

So, I expect that new Raw_Editable will have a similar structure with this. (Am I right?)

In this point, the way #1 looks correct to me.

How do you think about this ?

tasn added a comment.Sep 4 2019, 1:19 AM

@ali.alzyod - Will you work in "devs/tasn/ifaces" branch or make your own ?

I also want to check the modifications together.

Please work on your own branches (can be based on mine) and I'll pull them in as I go. I keep on messing with my branch and would rather not have to make sure I don't accidentally mess with your work.

tasn added a comment.EditedSep 4 2019, 1:20 AM

@tasn
Regard this task
Should we make our start point :

1- efl_ui_internal_interactive_text:

a- replace class name/data with efl_text_raw_edit
b- change events to use raw_editable events.
c- port missing edje_entry code to efl_text_raw_editable depending events/function/properties of current efl_text_raw_editable.eo file.

Or
2- Create new file from edje_entry

a- Generate efl_text_raw_edit headers/source files
b- Port evertthing from edje_entry to efl_text_raw_edit source files

I'd say definitely the second option. As I said above, don't think about merging in the internal one until you have a working/isolated edje_entry example. This will let us better understand what came from where in the future.

Edit: I was wrong, see the next comment.

tasn added a comment.Sep 4 2019, 1:26 AM

@tasn @ali.alzyod

If my understanding is correct -
"efl_ui_internal_text_interactive" extends "efl_canvas_text",
and has covered some features of edje_entry (except the features of EDJE)

So, I expect that new Raw_Editable will have a similar structure with this. (Am I right?)

In this point, the way #1 looks correct to me.

How do you think about this ?

WAIT, what. So apparently I created this object, and it looks like it's what I had in mind for raw_edit. I'm quite confused to be honest, because I don't remember doing it, and I missed it when reviewing the code a few weeks ago.

Correction: please just merge them as you see fit! Just make sure there is no Edje dependency.

woohyun added a comment.EditedSep 4 2019, 3:10 AM

@tasn

Thank you ! :) We just got the confirmation what we need to do next.

@ali.alzyod

As we talked - please go with following steps.

  1. list up features that we need to get from edje_entry
  2. make them as action items for this task
  3. set priorities among them
  4. do implementation (branch : devs/woohyun/raw_editable)
tasn added a comment.Sep 4 2019, 3:53 AM

Perfect. Yeah, I also had a chat on IRC with @ali.alzyod, my bad for misunderstanding him and this file. It already looks like it has most of what we need!

We collected features we could not find in :
T8151#141680

I will soon update Edje only version of these feature soon. (what is missing)

@woohyun These are main parts needs port from entry_edje:

  • Preedit Functinality (Missing Parts)
  • Anchor Functinality (Missing Parts)
  • SELECTION_CLEARED events (replace some of SELECTION_CHANGED)
  • Port missing Keyboard handling functionality
    1. Undo/Redo (I think there should be way to trigger them by code)
    2. Selection
  • Cursor_Manual_Change
  • Filters --This needs new design logic
  • Cursor down/up --This needs new design logic

Currently I am porting keyboard functinality

tasn added a comment.Sep 5 2019, 5:20 AM

Just port what's there, don't add missing parts. We'll afterwards adjust it to be more like what raw_edit is like.

Thanks a lot!

I mean missing parts in efl.ui.internal.interactivetext

woohyun added a comment.EditedSep 6 2019, 4:03 AM

@ali.alzyod

  • Anchor Functinality (Missing Parts)

-> This is now in efl_ui_text now (I don't think it's perfect yet.)

And, do we have item_provider support in current efl_ui_internal_text_interactive ?

@tasn
Where would be the proper location for this Anchor feature ?
efl_ui_text ? or raw_editable ?

Plus, currently, cursor object is drawn by efl_ui_text - Is it ok from your new design perspective ?

tasn added a comment.Sep 6 2019, 5:01 AM

@ali.alzyod

  • Anchor Functinality (Missing Parts)

-> This is now in efl_ui_text now (I don't think it's perfect yet.)

And, do we have item_provider support in current efl_ui_internal_text_interactive ?

item_provider is done differently at the moment, so you don't need it. Just to make sure we are talking about the same thing: we are talking about what translates an item to e.g. an emoji, correct?

@tasn
Where would be the proper location for this Anchor feature ?

efl_ui_text ? or raw_editable ?

I'm still trying to evaluate this one. Either here (if possible) or potentially make canvas_text aware of it (like now) if we ave no cohice.

Plus, currently, cursor object is drawn by efl_ui_text - Is it ok from your new design perspective ?

Yeah, that's perfect. The one controlling the cursor is raw_editable, but the one actually drawing it (because it has the theme information) is ui_text.

Changes are in D9838

tasn added a comment.Sep 6 2019, 6:58 AM

Is D9838 everything? Wow, that was quick, good job! :)

@tasn No it is not, we are updating/porting current Efl.Ui.Internal.Interactive.c file with edje_entry.c
Do we need to port filter functions like _text_filter_text_prepend or _text_filter_markup_prepend, ... etc?

tasn added a comment.Sep 6 2019, 7:10 AM

No, filters and validators are going to be implemented differently, so no need for those.

Anchor and Items ?
Like _anchors_update, _anchors_update_check, etc and Items functions like _edje_entry_item_geometry_get, _item_obj_del_cb, etc

tasn added a comment.Sep 6 2019, 7:16 AM

item_geometry_get: I don't remember what this one is exactly, but if memory serves it's just a way to get the item's size right? If so, not needed. _item_obj_del_cb: probably also not needed.

Anchors: I'd wait with those for now, as I'm not sure how we should implement them just yet. It's one of those decisions that I decided to punt, and decide in like a week or two when we have implementations actually forming.

ali.alzyod added a comment.EditedSep 6 2019, 8:28 AM

@tasn @woohyun , edje_entry have these functionality (missing in efl.ui.internal.interactive)

input_panel like _edje_entry_input_panel_layout_set,
prediction like _edje_entry_prediction_hint_set
autocapital like _edje_entry_autocapital_type_set

They internally uses ecore_imf_ functions, do we need to port them ?

@ali.alzyod
I think porting them is needed, but I'm not sure about the proper class which would support them.

@tasn
Do you think they are appropriate to be in Raw_Editable?

  • add panel_layout
  • add prediction
  • add autocapition
  • add imf_event

Code contains #warning preprocessor for new added code (that still need connection or change).
Code will not compile (some function ported but needs change so it can compile, I did main changes but still there are multiple part do not compile)

tasn added a comment.Sep 9 2019, 6:53 AM

Yeah, I agree with @woohyun. All of these look like they should be ported.

@tasn @woohyun
I see that a lot of stuff split between (efl.ui.internal.interactive and efl.ui.text) in what already founded in EFL.

(1) Do we need to port (cursor_bg, cursor_fg, cursor_fg2) from edje_entry ? (these options already in efl.ui.text)
(2) Password Timer ?
(3) cursor_user, *cursor_user_extra ?
(4) itemlist, item_objs ?
(5) anchors, anchorlist ? (If you decided what to do with them)

tasn added a comment.EditedSep 10 2019, 2:45 AM

@tasn @woohyun
I see that a lot of stuff split between (efl.ui.internal.interactive and efl.ui.text) in what already founded in EFL.

(1) Do we need to port (cursor_bg, cursor_fg, cursor_fg2) from edje_entry ? (these options already in efl.ui.text)

What are you referring to exactly? Just the theme handling? That will be in ui.text.

(2) Password Timer ?

This should be in raw_editable, though don't worry about moving the implementation of passwords at the moment, because that would be different.

(3) cursor_user, *cursor_user_extra ?

Don't worry about those, not needed.

(4) itemlist, item_objs ?
(5) anchors, anchorlist ? (If you decided what to do with them)

Wait for now. I think they will be driven by the implementation. Let's see what we can do with them.

Edit; Just one clarification: do what's easiest for you. If it's easier for you to also migrate those, you can migrate them and worst case we remove them later. I just assume it's easier not to migrate them, so you shouldn't worry about it.

For following event in RawEditable:
preedit,changed: void; Called when entry preedit changed
In legacy code this event expect info to be passed to it (not void)

tasn added a comment.Sep 17 2019, 6:01 AM

This is a mistake then, I guess I copy-pasted it wrong (are you sure it's the case in the old .eo files?). Anyhow, make it like legacy and we'll worst case review it at a later stage.

tasn added a comment.EditedSep 18 2019, 9:54 AM

One more comment about this task in case it's not already there: just work on grapheme clusters, not chars. Don't make this toggleable or anything, it should always work on grapheme clusters. This was requested by Samsung before, and it's a great idea and a good opportunity to fix this.

Edit: I'm talking about e.g. when someone is clicking the mouse, or the "right" arrow key.

ali.alzyod added a comment.EditedSep 22 2019, 11:27 PM

@tasn
(1)
While porting from edje_entry we notice missing cursor functions
evas_textblock_cursor_line_geometry_get
evas_textblock_cursor_cluster_coord_set
evas_textblock_cursor_line_coord_set
evas_textblock_cursor_cluster_next
evas_textblock_cursor_cluster_prev

(2)
Missing function in new design:
efl2_text_raw_editable_prediction_hint_hash_set
efl2_text_raw_editable_prediction_hint_hash_del
efl2_text_raw_editable_prediction_hint_set
efl2_text_raw_editable_select_all
efl2_text_raw_editable_select_abort

(3)
Missing Events in new design:
EFL2_TEXT_RAW_EDITABLE_EVENT_SELECTION_ALL
EFL2_TEXT_RAW_EDITABLE_EVENT_TEXT_PASTE
EFL2_TEXT_RAW_EDITABLE_EVENT_CHANGED

tasn added a comment.Sep 23 2019, 1:19 AM

@tasn
(1)
While porting from edje_entry we notice missing cursor functions
evas_textblock_cursor_line_geometry_get

It was only ever used to get the line number (now line_number_get). If you want to get the geometry of a line you can get a range geometry, but again, no one ever needed/used that.

evas_textblock_cursor_cluster_coord_set

This is now the default behaviour, we no longer allow setting within a cluster, so it's there. You can easily implement setting within a cluster externally though, it's just splitting the cluster width to how many chars there are, and then setting the char based on the offset.

evas_textblock_cursor_line_coord_set

As I already told you, this is only used once in a piece of code that's wrong and is trying to re-implement coord_set in edje_entry.c. I don't think it's needed.

evas_textblock_cursor_line_char_first
evas_textblock_cursor_line_char_last

Now called line_start/end.

evas_textblock_cursor_cluster_next
evas_textblock_cursor_cluster_prev

Now cluster_end followed by char_next, as is only ever used internally in edje_entry. If we have usecases where we need it often externally, I'm happy to add them. If you feel strongly about this, I'm also happy to add this.

(2)
Missing function in new design:
efl2_text_raw_editable_prediction_hint_hash_set
efl2_text_raw_editable_prediction_hint_hash_del
efl2_text_raw_editable_prediction_hint_set

Are they not in the interface? Where were they before, what do they do?

efl2_text_raw_editable_select_all

I'm open to adding this, though it's essentially select start of text to end of text.

efl2_text_raw_editable_select_abort

Isn't this just select_none? I didn't even rename it for all I remember, it was just what was there...

(3)
Missing Events in new design:
EFL2_TEXT_RAW_EDITABLE_EVENT_SELECTION_ALL

Don't see why this is needed.

EFL2_TEXT_RAW_EDITABLE_EVENT_TEXT_PASTE

Not sure what this would be useful for.

EFL2_TEXT_RAW_EDITABLE_EVENT_CHANGED

It's inherited from canvas.text, so there.

ali.alzyod added a comment.EditedSep 23 2019, 3:50 AM

@tasn Please note these functions/events are not suggestions, they are actually used in edje_entry but we do not have them in the new design.

evas_textblock_cursor_line_geometry_get
It was only ever used to get the line number (now line_number_get). If you want to get the geometry of a line you can get a range geometry, but again, no one ever needed/used that.

1- It is used for in edje_entry to get the geometry of the line, not line number only
2- no one ever needed/used that How do you know that? we internally use it, so it is logical other people use it too.
3- you can get a range geometry But I will need to create two cursors, then delete them after getting the range, is this efficient?

evas_textblock_cursor_cluster_coord_set
This is now the default behavior, we no longer allow setting within a cluster, so it's there. You can easily implement setting within a cluster externally though, it's just splitting the cluster width to how many chars there are, and then setting the char based on the offset.

Same as above, If we used it there, why do we assume user never use it, or implement it externally

evas_textblock_cursor_line_coord_set
As I already told you, this is only used once in a piece of code that's wrong and is trying to re-implement coord_set in edje_entry.c. I don't think it's needed.

Should I replace the code with efl2 coord_set ?

evas_textblock_cursor_cluster_next
evas_textblock_cursor_cluster_prev
Now cluster_end followed by char_next, as is only ever used internally in edje_entry. If we have usecases where we need it often externally, I'm happy to add them. If you feel strongly about this, I'm also happy to add this.

One of main rules of a cursor is iterate through the text, And I think the user will need such feature (as we need it internally)

(2)
Missing function in new design:
efl2_text_raw_editable_prediction_hint_hash_set
efl2_text_raw_editable_prediction_hint_hash_del
efl2_text_raw_editable_prediction_hint_set
Are they not in the interface? Where were they before, what do they do?

No they are not in the interface, As documentation says

/**
 * @brief Sets the prediction hint data at the specified key.
 *
 * @param[in] part The part name
 * @param[in] key The key of the prediction hint
 * @param[in] value The data to replace
 *
 * @return @c true on success, @c false otherwise
 *
 * @since 1.21.0
 *
 * @ingroup Edje_Object
 */
EAPI Eina_Bool edje_object_part_text_prediction_hint_hash_set(Evas_Object *obj, const char *part, const char *key, const char *value);

efl2_text_raw_editable_select_all
I'm open to adding this, though it's essentially select start of text to end of text.

It is already on edje_entry, and I think the user may benefit from it.

efl2_text_raw_editable_select_abort
Isn't this just select_none? I didn't even rename it for all I remember, it was just what was there...

I do not think they are the same, edje_entry have both, select_abort reset imf_context_reset()

(3)
Missing Events in new design:
EFL2_TEXT_RAW_EDITABLE_EVENT_SELECTION_ALL
Don't see why this is needed.

I will change it to selection_change event

EFL2_TEXT_RAW_EDITABLE_EVENT_TEXT_PASTE
Not sure what this would be useful for.

I will remove it.

EFL2_TEXT_RAW_EDITABLE_EVENT_CHANGED
It's inherited from canvas.text, so there.

Sorry I did not make this clear, efl.text.internal.interactive calls EFL_UI_TEXT_EVENT_CHANGED, I think we will keep it?

Also one missing Event I did not mention,
"selection,start" do we need this one ?

tasn added a comment.Sep 23 2019, 12:41 PM

@tasn Please note these functions/events are not suggestions, they are actually used in edje_entry but we do not have them in the new design.

evas_textblock_cursor_line_geometry_get
It was only ever used to get the line number (now line_number_get). If you want to get the geometry of a line you can get a range geometry, but again, no one ever needed/used that.

1- It is used for in edje_entry to get the geometry of the line, not line number only

Only in the place where it's re-implementing coord set in an awkward way inside of edje entry.

2- no one ever needed/used that How do you know that? we internally use it, so it is logical other people use it too.

I disagree with this statement, there are a lot of functions we only use internally and are not useful outside so we don't expose them as API. Actually, not a lot, the vast majority of functions are internal either to file or at least to binary.

3- you can get a range geometry But I will need to create two cursors, then delete them after getting the range, is this efficient?

You don't need to delete cursors... If you plan on using cursors on an object again and again, you can just keep them. If you only plan on doing things once, why do you care about "efficiency"?

evas_textblock_cursor_cluster_coord_set

Same as above, If we used it there, why do we assume user never use it, or implement it externally

you haven't read what I wrote. I'm saying the opposite, I'm saying the normal behaviour is what this one now does. Also, already refuted this statement above.

evas_textblock_cursor_line_coord_set
As I already told you, this is only used once in a piece of code that's wrong and is trying to re-implement coord_set in edje_entry.c. I don't think it's needed.

Should I replace the code with efl2 coord_set ?

This whole if statement should be removed. Please read what I wrote.

evas_textblock_cursor_cluster_next
evas_textblock_cursor_cluster_prev
Now cluster_end followed by char_next, as is only ever used internally in edje_entry. If we have usecases where we need it often externally, I'm happy to add them. If you feel strongly about this, I'm also happy to add this.

One of main rules of a cursor is iterate through the text, And I think the user will need such feature (as we need it internally)

I don't think you're right, but again, I'm not against it. I doubt people just go around doing cursor next on their own. I removed them because adding API that maybe someone will use when there are no actual users or examples is a key to adding useless API. This can already be implemented in a different way, we can always add the shortcut later if we later see that it's common. I'm relenting on this one, and will add it, though I don't think it's necessary.

(2)
Missing function in new design:
efl2_text_raw_editable_prediction_hint_hash_set
efl2_text_raw_editable_prediction_hint_hash_del
efl2_text_raw_editable_prediction_hint_set
Are they not in the interface? Where were they before, what do they do?

No they are not in the interface, As documentation says

/**
 * @brief Sets the prediction hint data at the specified key.
 *
 * @param[in] part The part name
 * @param[in] key The key of the prediction hint
 * @param[in] value The data to replace
 *
 * @return @c true on success, @c false otherwise
 *
 * @since 1.21.0
 *
 * @ingroup Edje_Object
 */
EAPI Eina_Bool edje_object_part_text_prediction_hint_hash_set(Evas_Object *obj, const char *part, const char *key, const char *value);

Even after reading the docs, I have no idea what they do.

Why were they not exposed in elm then?

efl2_text_raw_editable_select_all
I'm open to adding this, though it's essentially select start of text to end of text.

It is already on edje_entry, and I think the user may benefit from it.

Why was it not exposed in elm then? Again, it's very easy to implement if people need it.

efl2_text_raw_editable_select_abort
Isn't this just select_none? I didn't even rename it for all I remember, it was just what was there...

I do not think they are the same, edje_entry have both, select_abort reset imf_context_reset()

Not sure I see the distinction still. Maybe context_reset should be added to select_none? I don't know, just asking.

(3)
Missing Events in new design:
EFL2_TEXT_RAW_EDITABLE_EVENT_SELECTION_ALL
Don't see why this is needed.

I will change it to selection_change event

EFL2_TEXT_RAW_EDITABLE_EVENT_TEXT_PASTE
Not sure what this would be useful for.

I will remove it.

EFL2_TEXT_RAW_EDITABLE_EVENT_CHANGED
It's inherited from canvas.text, so there.

Sorry I did not make this clear, efl.text.internal.interactive calls EFL_UI_TEXT_EVENT_CHANGED, I think we will keep it?

No, it should use the same event as the canvas.text one.

Also one missing Event I did not mention,
"selection,start" do we need this one ?

What's already there in raw_editable:

selection,start: void; [[Called at selection start]]
selection,changed: void; [[Called when selection is changed]]
selection,cleared: void; [[Called when selection is cleared]]

So it's already there. Were you suggesting to remove it?

All changes ready in this pull request into your branch
https://github.com/Enlightenment/efl/pull/3

tasn added a comment.Oct 1 2019, 1:25 AM

Thanks a lot! Weird though, I thought we disabled PRs on the e github...

Anyhow, I'm pulling it into my branch, as I assume it's good. I haven't reviewed it yet.

tasn closed this task as Resolved.Oct 2 2019, 2:26 AM

Merged.

There's still work to be done, but the main parts are there.