[tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list

As requested, I went through all changes in #78 and used APPLY_IDL_PROPERTY() where possible. In some cases it wasn't possible, so there are a few handwritten setters. I also avoid any fancy new features in the code or ui, this is an 1:1 port.

This also fixes the C++ property loader code in bsestorage.cc so that make check passes.

From my perspective the only thing that could be controversial is that you used g_object_set(...property...value...) to avoid undo recording for some setter calls. I added a function to block undo manually so

  g_object_set (self, /* no undo */
               "sync", self->saved_sync,
                NULL);

now looks somewhat like this

  bus->block_property_undo(); 
  bus->sync (self->saved_sync);
  bus->unblock_property_undo();

This should work in the same way for setters that use APPLY_IDL_PROPERTY() and for hand-written setters that use push_property_undo(). If you have a better suggestion feel free to either change this after merging or letting me know how it should be done.


You can view, comment on, or merge this pull request online at:

  https://github.com/tim-janik/beast/pull/105

Commit Summary

  • BSE: Bus::mute: port property to C++
  • BEAST-GTK: bstbuseditor: support both, aida and old style properties
  • BSE: Bus::solo: port property to C++
  • BSE: bseitem: provide API to temporarily disable property undo recording
  • BSE: Bus::sync: port property to C++
  • BSE: Bus::left_volume: port property to C++
  • BEAST-GTK: bstbuseditor: use left_volume C++ property
  • BSE: Bus::right_volume: port property to C++
  • BEAST-GTK: bstbuseditor: use right_volume C++ property
  • BSE: Bus::master_output: port property to C++
  • SFI: provide scanner double/int64 parsing code
  • BSE: fix C++ property loading (float64)
  • BSE: bsebus: minor cosmetic changes
  • BSE: Song: port remaining Bus::master-output set calls

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105", "url": "https://github.com/tim-janik/beast/pull/105", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list

@tim-janik requested changes on this pull request.

Please fix review comments I left for you and remove:
a) the commit introducing the block/unblock API,
b) the float64 workaround.
If that means you updated PR is broken, point out the bugs you're runing into and I'll give you a hand with the fixes. In particualy, I'm not even sure you can trigger buggy behaviour due to not blocking undo, if you find a way to trigger that, that'd be really helpful. As for fixing the float parsing, I looked into that earlier and am quite confident I can fix parse_paren_rest. I just needed an actual test case that triggered breakage, which I didn't have at the time.


In bse/bsebus.cc:

> @@ -1003,6 +993,25 @@ BusImpl::BusImpl (BseObject *bobj) :
 BusImpl::~BusImpl ()
 {}
 
+bool
+BusImpl::mute() const
+{
+  BseBus *self = const_cast<BusImpl*> (this)->as<BseBus*>();
+
+  return self->muted;
+}
+
+void
+BusImpl::mute (bool val)
+{
+  BseBus *self = const_cast<BusImpl*> (this)->as<BseBus*>();
+
+  if (APPLY_IDL_PROPERTY (self->muted, val))
+    {

Please avoid excessive newlines and braces. This function has 3 lines of code but is pumped up to 6 lines in total. The following is prefectly readable:

  BseBus *self = const_cast<BusImpl*> (this)->as<BseBus*>();
  if (APPLY_IDL_PROPERTY (self->muted, val))
    bus_volume_changed (self);

In beast-gtk/bstbuseditor.cc:

> @@ -89,8 +89,24 @@ bus_build_param (BstBusEditor *self,
                  const gchar  *editor,
                  const gchar  *label)
 {
-  GParamSpec *pspec = bse_proxy_get_pspec (self->item, property);
-  self->params = sfi_ring_prepend (self->params, bst_param_new_proxy (pspec, self->item));
+  GxkParam *gxk_param = nullptr;

This should be moved down to be closest to the site of use.


In beast-gtk/bstbuseditor.cc:

> @@ -89,8 +89,24 @@ bus_build_param (BstBusEditor *self,
                  const gchar  *editor,
                  const gchar  *label)
 {
-  GParamSpec *pspec = bse_proxy_get_pspec (self->item, property);
-  self->params = sfi_ring_prepend (self->params, bst_param_new_proxy (pspec, self->item));
+  GxkParam *gxk_param = nullptr;
+
+  /* aida property? */
+  Bse::BusH bus = Bse::BusH::__cast__ (bse_server.from_proxy (self->item));

There is only a single caller for this function, so the cast should be moved up into the caller and avoided here.


In beast-gtk/bstbuseditor.cc:

> @@ -89,8 +89,24 @@ bus_build_param (BstBusEditor *self,
                  const gchar  *editor,
                  const gchar  *label)
 {
-  GParamSpec *pspec = bse_proxy_get_pspec (self->item, property);
-  self->params = sfi_ring_prepend (self->params, bst_param_new_proxy (pspec, self->item));
+  GxkParam *gxk_param = nullptr;
+
+  /* aida property? */
+  Bse::BusH bus = Bse::BusH::__cast__ (bse_server.from_proxy (self->item));
+  const Bse::StringSeq meta = bus.find_prop (property);

Instead of meta we've been using kvlist elsewhere.
The following lines should be wrapped into if (!meta.empty()) { ...pspec_from_key_value_list }
And bse_proxy_get_pspec should only be called if meta (kvlist) was empty.


In bse/bsebus.cc:

> +  if (BSE_IS_SONG (parent))
+    {
+      BseSong *song = BSE_SONG (parent);
+      return song->solo_bus == self;
+    }
+  return false;
+}
+
+void
+BusImpl::solo (bool is_solo)
+{
+  BseBus *self = as<BseBus*>();
+
+  if (solo() != is_solo)
+    {
+      auto prop = "solo";

This can be const.


In bse/bsebus.cc:

> @@ -842,9 +824,10 @@ bus_restore_finish (BseObject *object,
 {
   BseBus *self = BSE_BUS (object);
   /* restore real sync setting */
-  g_object_set (self, /* no undo */
-                "sync", self->saved_sync,
-                NULL);
+  auto impl = self->as<Bse::BusImpl*>();
+  impl->block_property_undo();
+  impl->sync (self->saved_sync);
+  impl->unblock_property_undo();

Please get rid of the block/unblock calls here, I think we need to handle that elsewhere.
This code is only called during restore(), and the result of the undo stack during restore is thrown away anyway (in ProjectImpl::restore_from_file(): bse_project_clear_undo). Actually we should be using bse_undo_stack_dummy during restore anyway.


In beast-gtk/bstbuseditor.cc:

> @@ -82,22 +82,28 @@ bus_editor_release_item (SfiProxy      item,
   bst_bus_editor_set_bus (self, 0);
 }
 
+static GxkParam *
+get_property_param (BstBusEditor *self,
+                    const gchar  *property)
+{
+  Bse::BusH bus = Bse::BusH::__cast__ (bse_server.from_proxy (self->item));
+  const Bse::StringSeq meta = bus.find_prop (property);
+
+  GParamSpec *cxxpspec = Bse::pspec_from_key_value_list (property, meta);

Use kvlist.


In beast-gtk/bstbuseditor.cc:

> @@ -82,22 +82,28 @@ bus_editor_release_item (SfiProxy      item,
   bst_bus_editor_set_bus (self, 0);
 }
 
+static GxkParam *
+get_property_param (BstBusEditor *self,
+                    const gchar  *property)
+{
+  Bse::BusH bus = Bse::BusH::__cast__ (bse_server.from_proxy (self->item));

We are moving to Aida handles everywhre, so always use ItemH, BusH, etc instead of proxies, i.e. simply pass a handle into this function.


In bse/bsebus.cc:

> +    {
+      BseSong *song = BSE_SONG (parent);
+      BseBus *master = bse_song_find_master (song);
+      return (self == master);
+    }
+  return false;
+}
+
+void
+BusImpl::master_output (bool val)
+{
+  BseBus *self = as<BseBus*>();
+
+  if (val != master_output())
+    {
+      auto prop = "master_output";

Add const


In bse/bsestorage.cc:

> @@ -741,6 +741,16 @@ storage_parse_property_value (BseStorage *self, const std::string &name, Bse::An
 {
   assert_return (BSE_IS_STORAGE (self), G_TOKEN_ERROR);
   GScanner *scanner = bse_storage_get_scanner (self);
+  if (any.kind() == Aida::FLOAT64) /* float format cannot be handled by scanner_parse_paren_rest */

I don't think any.kind() will always hold the desired type here for all code paths.
This is more a workaround than a fix, I think scanner_parse_paren_rest should be fixed instead.


In bse/bsebus.cc:

>    if (BSE_IS_SONG (parent))
     {
       BseSong *song = BSE_SONG (parent);
       BseBus *master = bse_song_find_master (song);
-      return (self == master);
+      return self == master;

Code like this can be merged down into the culprit of your current branch. Here's how you do that:

  1. Use git blame HEAD -- bsebus.cc to find the faulty commit:
aaabbbccc   bse/bsebus.c swesterfeld     return (self == master);
  1. Commit the fix as in:
git commit -m '+++ fix commit aaabbbccc' 
111222333
  1. Assuming your current branch started from master, merge this down with: git rebase -i master
  2. Move pick 111222333 +++ fix commit aaabbbccc into a squash or fixup following the culprit:
pick aaabbbccc BSE: introduce buggy code
squash 111222333 +++ fix commit aaabbbccc

In bse/bsesong.cc:

>        Bse::BusImpl *bus = child->as<Bse::BusImpl*>();
+      bus->block_property_undo();

Get rid of the block/unblock here, I'd like this to be handled elsewhere.


In bse/bsesong.cc:

> @@ -681,7 +683,7 @@ SongImpl::remove_bus (BusIface &bus_iface)
   BseItem *child = bus.as<BseItem*>();
   BseUndoStack *ustack = bse_item_undo_open (self, __func__);
   // reset ::master-output property undoable

The comment about undoable here can be removed.


In bse/bseitem.cc:

> @@ -1335,4 +1337,16 @@ ItemImpl::apply_idl_property_need_undo (const StringVector &kvlist)
   return !Aida::aux_vector_check_options (kvlist, "", "hints", "skip-undo");
 }
 
+void
+ItemImpl::block_property_undo()
+{

I really don't want to introduce these. We already have other mechanism that check if undo steps should be recorded at all, and we have a dummy undo stack bse_undo_stack_dummy that could be used if we'd want to temporarily ignore undo steps. Also, the C++ developer shouldn't be bothered with having to use block/unblock in pairs, so for the future, similar API should make use of ctor/dtor pairs like std::lock_guard does.

Assuming your branch starts off from master, you can get rid of this commit with git rebase -i and then remove the pick ... line that refers to this commit.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XVNKLPY76APW3T55PLPWP4FFA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZHD25Q#pullrequestreview-240008566", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XVNKLPY76APW3T55PLPWP4FFA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZHD25Q#pullrequestreview-240008566", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In beast-gtk/bstbuseditor.cc:

> @@ -89,8 +89,24 @@ bus_build_param (BstBusEditor *self,
                  const gchar  *editor,
                  const gchar  *label)
 {
-  GParamSpec *pspec = bse_proxy_get_pspec (self->item, property);
-  self->params = sfi_ring_prepend (self->params, bst_param_new_proxy (pspec, self->item));
+  GxkParam *gxk_param = nullptr;

Right. This has been refactored in a later commit, so the issue is effectively resolved.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XVTSNERJXX2LLPGS23PYET6FA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7R2Q#discussion_r289395958", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XVTSNERJXX2LLPGS23PYET6FA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7R2Q#discussion_r289395958", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsebus.cc:

> @@ -1003,6 +993,25 @@ BusImpl::BusImpl (BseObject *bobj) :
 BusImpl::~BusImpl ()
 {}
 
+bool
+BusImpl::mute() const
+{
+  BseBus *self = const_cast<BusImpl*> (this)->as<BseBus*>();
+
+  return self->muted;
+}
+
+void
+BusImpl::mute (bool val)
+{
+  BseBus *self = const_cast<BusImpl*> (this)->as<BseBus*>();
+
+  if (APPLY_IDL_PROPERTY (self->muted, val))
+    {

Ok, the lines caused by { } are not really necessary, so I removed then. I'd still like to keep a blank line between BseBus *self and the rest of the code. Obviously "readability" is not an objective criterium but depends on the coder reading the code. But for me, the extra blank line makes the code more readable. I often use blank lines to seperate code in groups that belong together. And we're no longer coding with 80x24 terminals, so we have space for many lines on the screen.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XWPFPHIC6P3NDTFUUDPYET6ZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7SNQ#discussion_r289396024", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XWPFPHIC6P3NDTFUUDPYET6ZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7SNQ#discussion_r289396024", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In beast-gtk/bstbuseditor.cc:

> @@ -89,8 +89,24 @@ bus_build_param (BstBusEditor *self,
                  const gchar  *editor,
                  const gchar  *label)
 {
-  GParamSpec *pspec = bse_proxy_get_pspec (self->item, property);
-  self->params = sfi_ring_prepend (self->params, bst_param_new_proxy (pspec, self->item));
+  GxkParam *gxk_param = nullptr;
+
+  /* aida property? */
+  Bse::BusH bus = Bse::BusH::__cast__ (bse_server.from_proxy (self->item));

I fixed this in a later commit.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XS7MFHI2C4K2GIGV6LPYEUCHA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7WEQ#discussion_r289396350", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XS7MFHI2C4K2GIGV6LPYEUCHA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7WEQ#discussion_r289396350", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In beast-gtk/bstbuseditor.cc:

> @@ -89,8 +89,24 @@ bus_build_param (BstBusEditor *self,
                  const gchar  *editor,
                  const gchar  *label)
 {
-  GParamSpec *pspec = bse_proxy_get_pspec (self->item, property);
-  self->params = sfi_ring_prepend (self->params, bst_param_new_proxy (pspec, self->item));
+  GxkParam *gxk_param = nullptr;
+
+  /* aida property? */
+  Bse::BusH bus = Bse::BusH::__cast__ (bse_server.from_proxy (self->item));
+  const Bse::StringSeq meta = bus.find_prop (property);

Fixed in a later commit.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XT23EWGE4HXVHXWIKTPYEUC5A5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7W4A#discussion_r289396411", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XT23EWGE4HXVHXWIKTPYEUC5A5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7W4A#discussion_r289396411", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsebus.cc:

> +  if (BSE_IS_SONG (parent))
+    {
+      BseSong *song = BSE_SONG (parent);
+      return song->solo_bus == self;
+    }
+  return false;
+}
+
+void
+BusImpl::solo (bool is_solo)
+{
+  BseBus *self = as<BseBus*>();
+
+  if (solo() != is_solo)
+    {
+      auto prop = "solo";

Fixed.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XXSLVTHI2ZQT745TMDPYEUDHA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7XIQ#discussion_r289396448", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XXSLVTHI2ZQT745TMDPYEUDHA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H7XIQ#discussion_r289396448", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsestorage.cc:

> @@ -741,6 +741,16 @@ storage_parse_property_value (BseStorage *self, const std::string &name, Bse::An
 {
   assert_return (BSE_IS_STORAGE (self), G_TOKEN_ERROR);
   GScanner *scanner = bse_storage_get_scanner (self);
+  if (any.kind() == Aida::FLOAT64) /* float format cannot be handled by scanner_parse_paren_rest */

Right I removed the code now, so make check fails, so this needs to be fixed (probably by you) now by fixing the parser.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XUVMYMBMGUR3UDCQCDPYEUGZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H73II#discussion_r289396805", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XUVMYMBMGUR3UDCQCDPYEUGZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2H73II#discussion_r289396805", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsebus.cc:

>    if (BSE_IS_SONG (parent))
     {
       BseSong *song = BSE_SONG (parent);
       BseBus *master = bse_song_find_master (song);
-      return (self == master);
+      return self == master;

Thanks for the hint, I normally don't need this while coding, but I tried to fix everything here that could be done by history rewriting using git rebase -i now.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XUCNOCVWQEAJEDMP7TPYEULNA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IAALQ#discussion_r289397286", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XUCNOCVWQEAJEDMP7TPYEULNA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IAALQ#discussion_r289397286", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsesong.cc:

>        Bse::BusImpl *bus = child->as<Bse::BusImpl*>();
+      bus->block_property_undo();

Ok, I removed the block/unblock stuff now.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XUJSMAF3DUGDOPUSYDPYEUNFA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IACNQ#discussion_r289397477", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XUJSMAF3DUGDOPUSYDPYEUNFA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IACNQ#discussion_r289397477", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsesong.cc:

> @@ -681,7 +683,7 @@ SongImpl::remove_bus (BusIface &bus_iface)
   BseItem *child = bus.as<BseItem*>();
   BseUndoStack *ustack = bse_item_undo_open (self, __func__);
   // reset ::master-output property undoable

Fixed.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XTRUG5IYRFWTEHO5U3PYEUNZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IADCI#discussion_r289397537", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XTRUG5IYRFWTEHO5U3PYEUNZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IADCI#discussion_r289397537", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bseitem.cc:

> @@ -1335,4 +1337,16 @@ ItemImpl::apply_idl_property_need_undo (const StringVector &kvlist)
   return !Aida::aux_vector_check_options (kvlist, "", "hints", "skip-undo");
 }
 
+void
+ItemImpl::block_property_undo()
+{

Ok, I just tried to do a 1:1 port so I had to do something where the old code blocked undo. If you say you believe it is not necessary, thats even better, because the code looks cleaner now. I removed block/unblock now.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XQXGKHRPCAPSNMZBXTPYEUYVA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IAQBY#discussion_r289398764", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XQXGKHRPCAPSNMZBXTPYEUYVA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2IAQBY#discussion_r289398764", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro (#105)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

Please fix review comments I left for you and remove:
a) the commit introducing the block/unblock API,

Done with all code that depends on it.

b) the float64 workaround.

Done.

If that means you updated PR is broken, point out the bugs you're runing into and I'll give you a hand with the fixes. In particualy, I'm not even sure you can trigger buggy behaviour due to not blocking undo, if you find a way to trigger that, that'd be really helpful.

I haven't ever seen any undo related problem while testing it.

As for fixing the float parsing, I looked into that earlier and am quite confident I can fix parse_paren_rest. I just needed an actual test case that triggered breakage, which I didn't have at the time.

Right. make check is broken now due to audio tests, so before merge, you should look into fixing the parser.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XQZUWLK24LFUFEDY3DPYEVBZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWVJF5Y#issuecomment-497718007", "url": "https://github.com/tim-janik/beast/pull/105?email_source=notifications\u0026email_token=AIVS7XQZUWLK24LFUFEDY3DPYEVBZA5CNFSM4HK3SEB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWVJF5Y#issuecomment-497718007", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast