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 the mixed-use of UUID and str for task IDs #1102

Merged
merged 1 commit into from
May 20, 2024

Conversation

gycsaba96
Copy link
Contributor

Fixes #1077

The cause of the (seemingly) extra empty line was that existing subtasks were not correctly detected and thus moved to the end of the text. This happened because of the mixed use of strings and UUIDs for task IDs.

This PR converts strings to UUIDs when necessary and fixes the related type hints.

The use of str instead of UUID caused problems during lookup
(e.g., not detecting existing subtasks correctly).

This fixes GitHub issue getting-things-gnome#1077
@SqAtx
Copy link
Contributor

SqAtx commented May 19, 2024

Thank you for looking at it! I kind of got bogged down in #1083 on my way to add tests for this part of the code ^^'

I see that this fixes the bug, but I don't understand why. How were the subtasks not correctly detected? Weren't they detected by line.lstrip().startswith('{!'):, which would be the same whether we use UUID or str?

@gycsaba96
Copy link
Contributor Author

Thank you for looking at it! I kind of got bogged down in #1083 on my way to add tests for this part of the code ^^'

Don't give up, QA is very important. :)

I see that this fixes the bug, but I don't understand why. How were the subtasks not correctly detected? Weren't they detected by line.lstrip().startswith('{!'):, which would be the same whether we use UUID or str?

Let me provide more details.

  1. When the subtask is created, it is stored in the task store's lookup dictionary with a UUID key.
  2. Line 772 and 777 extract and store the subtask's id as a string.
  3. Line 791 raises a KeyError because we want to do the lookup using a string key, although the subtask does exist in the store with a UUID key. (Sorry for referencing the wrong line.)
  4. As a consequence, it remains in the subtasks list and gets erased from the text by the for loop at line 801. This is how it is removed from the original position.
  5. Finally, it gets added back to the end of the text by editor.py#210 when you reopen the editor.

@diegogangl diegogangl added the bug label May 20, 2024
@diegogangl diegogangl merged commit 28c472f into getting-things-gnome:master May 20, 2024
1 check passed
@diegogangl
Copy link
Contributor

Merged, thanks!

@SqAtx
Copy link
Contributor

SqAtx commented May 25, 2024

@gycsaba96 ahhh I see, thank you! I haven't quite wrapped my head around the entire task/editor logic yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reopening a task with children adds a blank line before the list of children
3 participants