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

Fix crash when setting word Goal on new Text (scene) in Outline pane #609

Merged
merged 1 commit into from Jul 31, 2019

Conversation

gedakc
Copy link
Collaborator

@gedakc gedakc commented Jul 23, 2019

See issue #561.

The problem appears to be a due to a combination of factors, such as:

  • Python does not automatically convert an empty/blank variable to the
    integer zero (0)
  • Default goal value is empty/blank for a new Text (scene)
  • Asynchronous events can occur such that the change in the Outline
    pane of a new Text (scene) goal from empty/blank to a value is not
    saved to the data model prior to the update event in the Editor pane
    accessing the model value for the word count progress display.

Steps to Reproduce:

  1. Start manuskript and create new project (no template).

  2. Select Outline pane.

  3. Click "Text Plus" icon to create a text (default name "New")

  4. Select Editor pane.

  5. Click on New to display empty text.

  6. Select Outline pane.

  7. Double-click the empty area on New line under title Goal,
    type in "300", and press Enter.

    Note that manuskript crashes with a segmentation fault.

Work around the crash by using the already existing manuskript
function toInt() which handles conversion of empty/blank values to
integer value zero (0).

See issue olivierkes#561.

The problem appears to be a due to a combination of factors, such as:

- Python does not automatically convert an empty/blank variable to the
  integer zero (0)
- Default goal value is empty/blank for a new Text (scene)
- Asynchronous events can occur such that the change in the Outline
  pane of a new Text (scene) goal from empty/blank to a value is not
  saved to the data model prior to the update event in the Editor pane
  accessing the model value for the word count progress display.

Steps to Reproduce:

1. Start manuskript and create new project (no template).

2. Select **Outline** pane.

3. Click "Text Plus" icon to create a text (default name "New")

4. Select **Editor** pane.

5. Click on **New** to display empty text.

6. Select **Outline** pane.

7. Double-click the empty area on **New** line under title **Goal**,
  type in "300", and press **Enter**.

   Note that manuskript crashes with a segmentation fault.

Work around the crash by using the already existing manuskript
function toInt() which handles conversion of empty/blank values to
integer value zero (0).
@gedakc
Copy link
Collaborator Author

gedakc commented Jul 23, 2019

This PR is available for review and comments.

@gedakc gedakc merged commit 5f9ea3b into olivierkes:develop Jul 31, 2019
@gedakc gedakc deleted the issue561-outline-crash branch July 31, 2019 16:46
This was referenced Jul 31, 2019
@gedakc gedakc added this to the 0.10.0 milestone Aug 28, 2019
worstje added a commit to worstje/manuskript that referenced this pull request Sep 22, 2019
A previous fix (5f9ea3) inadvertently broke the progress bar by
converting to the wrong data type. (See issue olivierkes#561 / PR olivierkes#609).

While checking the code I realized the problem occurred primarily
because we weren't checking the validity of the values closer to the
source. Doing so alleviates the need to check elsewhere.

In the hope of inspiring a more systematic approach, a new uiParse()
utility function has been added to curb the further growth of toXxx()
functions that exist solely to validate user input.

There is no doubt room for improvement, both on the end of the new
uiParse() function as well as the spot where it is used. Ideally, the
data that comes out of the model should already be 'safe', but since
this is a bugfix for a bugfix I want to keep waves to a minimum.

This commit fixes issue olivierkes#652.
gedakc pushed a commit that referenced this pull request Sep 23, 2019
A previous fix (5f9ea3) inadvertently broke the progress bar by
converting to the wrong data type. (See issue #561 / PR #609).

While checking the code I realized the problem occurred primarily
because we weren't checking the validity of the values closer to the
source. Doing so alleviates the need to check elsewhere.

In the hope of inspiring a more systematic approach, a new uiParse()
utility function has been added to curb the further growth of toXxx()
functions that exist solely to validate user input.

There is no doubt room for improvement, both on the end of the new
uiParse() function as well as the spot where it is used. Ideally, the
data that comes out of the model should already be 'safe', but since
this is a bugfix for a bugfix I want to keep waves to a minimum.

This commit fixes issue #652.
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

1 participant