[tim-janik/beast] BSE: fix undo problems that occur after removing a bus (#82)

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

[tim-janik/beast] BSE: fix undo problems that occur after removing a bus (#82)

Gnome - Beast mailing list

To fix the problem, ensure that an appropriate undo step is always recorded.

This is a fix for issue #79


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

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

Commit Summary

  • BSE: fix undo problems that occur after removing a bus

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/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"BSE: fix undo problems that occur after removing a bus (#82)"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/82"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/82", "url": "https://github.com/tim-janik/beast/pull/82", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "BSE: fix undo problems that occur after removing a bus (#82)", "sections": [ { "text": "", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ ] }, { "title": "Commit Summary", "facts": [ { "name": "e10c9ee", "value": "BSE: fix undo problems that occur after removing a bus" } ] }, { "title": "File Changes", "facts": [ { "name": "Modified", "value": "[bse/bsebus.cc](https://github.com/tim-janik/beast/pull/82/files#diff-0) (7 changes)" } ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 82,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 82\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/82" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/82.patch" } ], "@type": "OpenUri", "name": "View patch" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/82.diff" } ], "@type": "OpenUri", "name": "View diff" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 380351199\n}" } ], "themeColor": "26292E" } ]</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] BSE: fix undo problems that occur after removing a bus (#82)

Gnome - Beast mailing list

Hey Stefan, thanks for looking into this.
I don't fully follow the argument you make in your comment though.

  • Why should an undo step be recorded for restoring a connection that doesn't exist in the first place?
  • Also, with your code, calling disconnect_bus() repeatedly will record more and more undo steps, for a connection that's not existing and could have been restored at most only once anyway.

It look like you're onto something here, but AFAIU the "fix" in this PR is almost certainly not the right one.


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/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@tim-janik in #82: Hey Stefan, thanks for looking into this.\r\nI don't fully follow the argument you make in your comment though.\r\n\r\n* Why should an undo step be recorded for restoring a connection that doesn't exist in the first place?\r\n* Also, with your code, calling disconnect_bus() repeatedly will record more and more undo steps, for a connection that's not existing and could have been restored at most only once anyway.\r\n\r\nIt look like you're onto something here, but AFAIU the \"fix\" in this PR is almost certainly not the right one."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/82#issuecomment-425030568"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/82#issuecomment-425030568", "url": "https://github.com/tim-janik/beast/pull/82#issuecomment-425030568", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [tim-janik/beast] BSE: fix undo problems that occur after removing a bus (#82)", "sections": [ { "text": "", "activityTitle": "**Tim Janik**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@tim-janik", "facts": [ ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 82,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 82\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/82#issuecomment-425030568" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 380351199\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast