Skip to content

Tool refactoring to reduce code duplication #148

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

Closed
TrueDoctor opened this issue May 23, 2021 · 2 comments · Fixed by #301
Closed

Tool refactoring to reduce code duplication #148

TrueDoctor opened this issue May 23, 2021 · 2 comments · Fixed by #301
Milestone

Comments

@TrueDoctor
Copy link
Member

TrueDoctor commented May 23, 2021

Restructure how the tool handles internal states such as currently pressed modifier keys. And we might need a way to share code between tools to reduce the amount of code duplication.

See also the related issue #151.

Complexity: 2
Involves: Editor (Rust)

@Keavon
Copy link
Member

Keavon commented Jun 17, 2021

@pkupper I believe this is a continuation of work you began when you did your first tool-related tasks. Is that correct? If this task wasn't your idea, I may be attributing my memory to the wrong person, sorry if that's the case.

This is soft-blocking work on additional tools, I believe (but correct me if I'm wrong). Could you update on the status of what this task entails and how important it is for working on new tools like the Pen Tool, Path Tool, and Text Tool? I know you are busy with #140 and #145 presently, but would you also be able to tackle and complete this over the weekend so work can begin on the crucial Path Tool (#82) and Pen Tool (#185)?

@pkupper pkupper self-assigned this Jun 17, 2021
@Keavon
Copy link
Member

Keavon commented Jul 3, 2021

Some loose notes on this subject from @TrueDoctor:

I would suggest unifying the approach for keeping internal states by having one "macro state" and modifiers e.g.

(Ready, Modifiers{center, keep_aspect_ratio})

with something like:

(state, mod, Message::LockAspectRatio) => {mod.aspect_ratio = true; state}

that should reduce some of the code duplication

we will probably still need a better approach for the make_operation function

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 a pull request may close this issue.

3 participants