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

Issue192 paired symbols, issue199 yaml support #231

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Malarg
Copy link
Collaborator

@Malarg Malarg commented Apr 26, 2023

resolves #192
resolves #199
resolves #235

@Malarg Malarg requested a review from alexeyinkin April 26, 2023 11:14
@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #231 (ef71caf) into main (b4e7714) will decrease coverage by 0.07%.
The diff coverage is 71.42%.

❗ Current head ef71caf differs from pull request most recent head 962aae2. Consider uploading reports for the commit 962aae2 to get more accurate results

@@            Coverage Diff             @@
##             main     #231      +/-   ##
==========================================
- Coverage   81.96%   81.89%   -0.07%     
==========================================
  Files         107      108       +1     
  Lines        3166     3187      +21     
==========================================
+ Hits         2595     2610      +15     
- Misses        571      577       +6     
Impacted Files Coverage Δ
lib/src/code_field/code_controller.dart 81.48% <ø> (ø)
lib/src/folding/parsers/parser_factory.dart 87.50% <50.00%> (-12.50%) ⬇️
lib/src/code_modifiers/insertion.dart 73.68% <73.68%> (ø)

lib/src/code_modifiers/paired_symbols.dart Outdated Show resolved Hide resolved
lib/src/code_field/code_controller.dart Outdated Show resolved Hide resolved
@Malarg Malarg requested a review from alexeyinkin May 11, 2023 07:23
lib/src/code_field/code_controller.dart Outdated Show resolved Hide resolved
lib/src/code_field/code_controller.dart Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

We need two tests:

  1. A unit test to InsertionCodeModifer with 1 and 23 strings.
  2. A test on CodeController that if created with the default parameters it handles all 6 pairs of characters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@Malarg Malarg requested a review from alexeyinkin May 22, 2023 07:30
@@ -23,6 +23,10 @@ class PopupController extends ChangeNotifier {

int get selectedIndex => _selectedIndex;

void reset() {
itemScrollController = ItemScrollController();
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the desired effect of this? Scroll to the top? If so, can you manually scroll to the top?

Replacing the controller has the drawbacks:

  • Need to dispose the old one.
  • Need to make sure the disposed one will not show in a widget and break it.

import 'package:flutter_test/flutter_test.dart';

void main() {
test('inserts at the start of string correctly', () {
Copy link
Contributor

Choose a reason for hiding this comment

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

group('InsertionCodeModifier', ...


void main() {
test('inserts at the start of string correctly', () {
const modifier = InsertionCodeModifier(openChar: '1', closeString: '23');
Copy link
Contributor

Choose a reason for hiding this comment

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

Make global?

const modifier = InsertionCodeModifier(openChar: '1', closeString: '23');
const text = 'Hello World';
final selection = TextSelection.fromPosition(const TextPosition(offset: 0));
const editorParams = EditorParams();
Copy link
Contributor

Choose a reason for hiding this comment

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

Make global?

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