Ticket #825 (closed planned_task: obsolete)

Opened 14 years ago

Last modified 13 years ago

TEXT_STABLE_SELECTION_R0

Reported by: Astea Owned by: vlado
Priority: 3 Milestone: M05_PRE5
Component: TEXT_FLOWING Version: 2.0
Keywords: Cc:
Category: EXTRA Effort: 0.5
Importance: 0 Ticket_group:
Estimated Number of Hours: Add Hours to Ticket:
Billable?: Total Hours:
Analysis_owners: deyan, nenko,deyan,boyan Design_owners: nenko, diana
Imp._owners: Test_owners:
Analysis_reviewers: dido Changelog:
Design_reviewers: meddle, pap, meddle Imp._reviewers: meddle, meddle, meddle, meddle
Test_reviewers: Analysis_score: 4
Design_score: 3 Imp._score: 3
Test_score: 0

Description

wiki page: TEXT_STABLE_SELECTION_R0 - effort: 0.5d

Attachments

selection_error.txt (1.2 MB) - added by meddle 13 years ago.
AssertionError with your implementation

Change History

comment:1 Changed 14 years ago by dido

  • Analysis_score set to 0
  • Test_score set to 0
  • Design_score set to 0
  • Owner changed from Astea to dido
  • Imp._score set to 0
  • Analysis_owners set to dido

comment:2 Changed 13 years ago by deyan

  • Owner changed from dido to deyan
  • Status changed from new to s1a_analysis_started
  • Analysis_owners changed from dido to deyan

comment:3 Changed 13 years ago by nenko

  • Status changed from s1a_analysis_started to s1b_analysis_finished
  • Analysis_owners changed from deyan to deyan, nenko

comment:4 Changed 13 years ago by dido

  • Status changed from s1b_analysis_finished to new
  • Analysis_score changed from 0 to 2.5

Analysis is fine but I have only one objection here.

  • Should apply style to selection only. If nothing is selected, should apply styles from cursor position to the next character which has own style.

- The behavior in most application is to set style only to the word that contains the current position. If some characters contains custom styles (for example if in EXAMPLE' the first E is underlined and the X is bold), when the user set current position between M and P, and then change the size, all characters in "EXAMPLE" should change this property. However if he press bold, what should happen is that entire word 'EXAMPLE' should be bold.

  • Don't know if this is for this task, but if I navigate using arrows and press ctrl key during that - I'll expect to position at the beginning of the next or previous word. However if I press shift key during that the behavior is different.
    • First case is to be in the beginning of the word and pres left arrow + ctrl + shift - this will select everything between current position and the beginning of the previous word(as expected)
    • Second case is to be in the beginning of the word and press right arrow + ctrl + shitf - this will select only the word and will set the current position at the end of the word.
    • As you can guess there should be a similar behavior if the initial position is in the end of a word.
    • Much more easy to explain case is when the initial position is in the middle(or in) a word. Then the selection is expected to be the remaining part of the word.
  • Last case that doesn't present in the analysis is when I have a text selection and press left or right arrow. The current position should be placed in the beginning of the selection(for left arrow) or at the end(for right arrow). Please provide a solution for navigating with arrows in rotated text(Probably we should ignore the rotation and navigate as the text os not rotated ).


analysis review 2.5p (1h)

comment:5 Changed 13 years ago by dido

  • Analysis_reviewers set to dido

comment:6 Changed 13 years ago by dido


  • Defining word in sens of where it begins and end could be following:
    • Characters like !@#$%&*()_+-="';:,.<>?/`~\| and space character sets beginning and end of a word.
    • However !@#$ is word.
    • EXA!@#PLE - are 3 words EXA !@# PLE
    • Space character and Tab character are not a word, even if they are one after another.

comment:7 Changed 13 years ago by deyan

  • Status changed from new to s1a_analysis_started
  • By character with own style i mean default style too. This is now cleared.
  • Navigating with arrows is not part of this task, but it is also clarified
  • Dropping selection is explained again

comment:8 Changed 13 years ago by deyan

  • Status changed from s1a_analysis_started to s1b_analysis_finished
  • By character with own style i mean default style too. This is now cleared.
  • Navigating with arrows is not part of this task, but it is also clarified
  • Dropping selection is explained again

comment:9 Changed 13 years ago by deyan

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_owners changed from deyan, nenko to deyan, nenko,deyan

comment:10 Changed 13 years ago by dido

  • Analysis_score changed from 2.5 to 3

comment:11 Changed 13 years ago by nenko

  • Design_owners set to nenko
  • Owner changed from deyan to nenko
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:12 Changed 13 years ago by diana

  • Status changed from s2a_design_started to s2b_design_finished

comment:13 Changed 13 years ago by pap

  • Status changed from s2b_design_finished to s1c_analysis_ok

Super review requested by nenko as the design is not actually finished.

comment:14 Changed 13 years ago by diana

  • Owner changed from nenko to diana
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:15 Changed 13 years ago by diana

  • Status changed from s2a_design_started to s2b_design_finished

comment:16 Changed 13 years ago by meddle

  • Design_owners changed from nenko to nenko, diana

comment:17 Changed 13 years ago by meddle

  • Status changed from s2b_design_finished to s1c_analysis_ok
  • Design_score changed from 0 to 2
  • Design_reviewers set to meddle, pap
  • Ticket related notes:
    • Fill your custom fields! The design owner is you - diana, I'm doing it for you now, but is your responsibility to do it yourself.
  • Design related notes:
    • What do you mean by "are not implemented" for some of the requirements? You design you talk about future implementation.
    • You can not decide to not implement requirements, the requirements are mandatory. If you want to shrink the scope the task should be super-reviewed to status 'new' and new analysis should be written with new requirements!
  • Code notes:
    • Is a good idea strings that you use to test like "Lorem ipsum dolor sit amet, consectetur adipiscing elit. " in your test to be exported to variables.
    • You have some progress on your code quality stuff but see again GoodCodeExamples. Spacing? And for n-th time assertEquals methods have special parameters. The first is the EXPECTED value, and the second is the ACTUAL.
    • And again if you have some model classes used in the test you MUST add them to the changesets in the design phase.

2p (70m)

comment:18 Changed 13 years ago by boyan

  • Status changed from s1c_analysis_ok to new

super reviewing to status new

comment:19 Changed 13 years ago by boyan

  • Owner changed from diana to boyan
  • Status changed from new to s1a_analysis_started
  • Analysis_owners changed from deyan, nenko,deyan to deyan, nenko,deyan,boyan

starting reanalysing (dropping some requirements)

comment:20 Changed 13 years ago by boyan

  • Status changed from s1a_analysis_started to s1b_analysis_finished

finished in about 15m

comment:21 Changed 13 years ago by dido

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_score changed from 3 to 4

Analysis is good enough for first revision
Review analysis 4p (1h)

comment:22 Changed 13 years ago by diana

  • Owner changed from boyan to diana
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:23 Changed 13 years ago by diana

  • Status changed from s2a_design_started to s2b_design_finished

comment:24 Changed 13 years ago by meddle

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 2 to 3
  • Design_reviewers changed from meddle, pap to meddle, pap, meddle
  • Code related notes:
    • I took a look at your "SelectPair" class and I was amazed... Why did you gave this class for the review?? It's full of bad code and the word for it is ugly. If this review wasn't in English I could express myself better but... I've got mental disorder...
  • About the design... I think it passes, but I didn't understand from it where is the "selectedUnits" property or if there is such one at all...

3p (25m)

comment:25 Changed 13 years ago by diana

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:26 Changed 13 years ago by diana

  • Status changed from s3a_implementation_started to s3b_implementation_finished

Changed 13 years ago by meddle

AssertionError with your implementation

comment:27 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 0 to 1
  • Imp._reviewers set to meddle

There are many problems with that implementation:

  • First of all your code is not working with the application because of the error in the attachment.
  • Your test is not put in the branch correctly and of course is unmergeable.
  • Run maven build and true runners first.
  • I don't understand how you can select anything when the 'shift' key is not working properly in sophie at this moment. How did you test your code?
  • You didn't read my design comments. Don't do that. Even if I pass designs they are important for the implementation.
  • I think you will have to update/reimplement your logic in new branch.
  • See that code : http://pastebin.asteasolutions.net/f4a4e50a8
    • The code for operation as select, delete, etc must look like that, simple.
    • I mean take out your repeating logic in such methods.

1p (65m)

comment:28 Changed 13 years ago by deyan

The character makes next comments unreadable...

comment:29 Changed 13 years ago by diana

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:30 Changed 13 years ago by diana

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:31 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 1 to 2.5
  • Imp._reviewers changed from meddle to meddle, meddle, meddle

I'll fail this task automatically because it, the chaining tasks and the paragraph tasks are in big conflicts... I will review it with the paragraph task in the same times, because vlado will merge the two and fix the conflicts, because they are not trivial... The problem is not in the conflicts when merging, it's from the behavior with the chaining code, so the task will be merged very soon but together with the paragraphs...

May be when doing such connected tasks you will have to give me some sequence of them...

2.5p (1h)

comment:32 Changed 13 years ago by vlado

  • Owner changed from diana to vlado
  • Status changed from s2c_design_ok to s3a_implementation_started

comment:33 Changed 13 years ago by vlado

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:34 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 2.5 to 3
  • Imp._reviewers changed from meddle, meddle, meddle to meddle, meddle, meddle, meddle

Code problems:

  • How many times I'll will have to tell you about the identation problem, I'll not let ugly code go to the trunk. I'll fix that now, but next time I'll fail tasks for ugly code! Ctrl+i in eclipse fixes the identation of selected code.
  • Java asserts in JUnit tests again :(((((((((((
  • Spacing!!! I'm glad that code can not be send directly into the trunk :)

And selecting words works differently from your implementation : if a word is selected and you try to select another one, the selection skips all the separators and goes to the end of the other one... I implemented that.

3p (80m)

comment:35 Changed 13 years ago by deyan

  • Status changed from s3c_implementation_ok to closed
  • Resolution set to obsolete

Batch update from file query-obsoleted.csv

Note: See TracTickets for help on using tickets.