Skip to content

State attempt#1

Open
worldsayshi wants to merge 10 commits intodevfrom
state-attempt
Open

State attempt#1
worldsayshi wants to merge 10 commits intodevfrom
state-attempt

Conversation

@worldsayshi
Copy link
Copy Markdown
Owner

@worldsayshi worldsayshi commented Apr 23, 2025

This is an attempt at improving state management. I am quite uncertain if this actually improves the code. Feels like it adds quite a lot of complexity. The idea is to adhere more to the guidelines.

@worldsayshi worldsayshi requested a review from Copilot April 23, 2025 19:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR aims to improve the state management mechanism throughout the application by centralizing state updates, introducing atomic flags to prevent recursive updates, and refactoring getter/setter methods for thread safety.

  • Update state management in internal/state/state.go to use atomic flags and explicit setter/getter methods.
  • Refactor application and test files to use the new state interface instead of direct accesses.
  • Enhance input component behavior to prevent infinite update loops.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
version.txt Updated version and parent commit references.
internal/state/state.go Refactored state management with atomic flags and new state methods.
internal/components/input.go Modified input behavior to prevent spurious change events.
application_test.go Updated test case to use state getter methods instead of direct state access.
application.go Refactored application initialization and event handling to use centralized AppState.
.github/copilot-instructions.md Added composability rules documentation.
Files not reviewed (1)
  • .vscode/launch.json: Language not supported
Comments suppressed due to low confidence (1)

internal/state/state.go:20

  • [nitpick] Consider renaming 'inUpdateFunc' to 'isUpdating' to improve clarity and consistency with the 'isInUpdate()' method.
inUpdateFunc   atomic.Bool // Flag indicating we're inside an Update call

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.

2 participants