Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#3552 closed defect (fixed)

Complete the implementation of moveClass API

Reported by: Adeel Asghar Owned by: Adrian Pop
Priority: high Milestone: 1.9.4
Component: Interactive Environment Version:
Keywords: Cc:

Description

The implementation of moveClass API seems incomplete. Also it would nice to have a direction parameter as boolean e.g true means move the class up and false means move the class down.

New APIs like MoveClassToTop and MoveClassToBottom are also required.
OMEdit can now generate package.order file but to make it useful these APIs are needed.

Change History (12)

comment:1 by Per Östlund, 9 years ago

How should moveClass handle non-class elements? When you want to move a class inside another class you might have components, extends, and other kinds of elements. Should moveClass ignore these when moving a class?

comment:2 by Adeel Asghar, 9 years ago

How do you mean? moveClass only moves the class within a package so i guess non-class elements can simply be ignored here. But maybe you are thinking something else and I didn't understand your question properly.

comment:3 by Per Östlund, 9 years ago

I didn't see anything about the class to be moved being in a package, so I just assumed that the parent class could be any kind of class. But even packages can contain e.g. constants, extends, etc. But I'll just ignore those then.

comment:4 by Adeel Asghar, 9 years ago

Yeah you are right. The description is not clear enough because I thought Adrian will fix it and I talked with him on Skype :).

I will give an example here what we need. Consider a following package,

package P
  model M1;
  end M1;

  model M2;
  end M2;

  model M3;
  end M3;
end P;

Now OMEdit has a context menu items move up, move down, move to top and move to bottom which allows moving classes with a package and also generates package.order file accordingly. But when we call list(P) the contents of the package are not changed. So we need an API moveClass(P.M2, "up") which should update the AST and calling list(P) should result in,

package P
  model M2;
  end M2;

  model M1;
  end M1;

  model M3;
  end M3;
end P;

comment:5 by Per Östlund, 9 years ago

Ok, I've changed moveClass to take an integer offset instead, so e.g. moveClass(A, 2) means to move A two steps down the list of classes. It's not much harder to implement and much more flexible.

I've already implemented the easy case, where the class is a top-level class (in which case we only have a list of classes). The case where the class resides in another class is a bit tricker, since we can then have all kinds of elements which are also divided into different public/protected sections.

comment:6 by Per Östlund, 9 years ago

After spending some time trying to figure out why I was loosing some classes I realised that getClassNames only lists public classes. I guess that means that you also are only interested in public classes, so moveClass should ignore protected classes too while moving a class? That is, if you have a package like this:

package P
  model A end A;
  model B end B;
protected
  model C end C;
  model D end D;
public
  model E end E;
end P;

Then moveClass(B, 1) should move B to after E, correct?

comment:7 by Adeel Asghar, 9 years ago

getClassNames lists the public classes by default. But OMEdit always use getClassNames(showProtected=true) and allows users to show/hide the protected classes through a settings flag. So I am interested in both public and protected classes which means moveClass(B, 1) should move B to after C and the contents of the package should become,

package P
  model A end A;
protected
  model C end C;
public
  model B end B;
protected
  model D end D;
public
  model E end E;
end P;

comment:8 by Per Östlund, 9 years ago

Resolution: fixed
Status: newclosed

Fixed in b7b4231, test case in da7ee23. moveClass now takes an integer offset, and uses magic to handle moves across sections.

comment:9 by Per Östlund, 9 years ago

Resolution: fixed
Status: closedreopened

Forgot about move to top and bottom.

comment:10 by Per Östlund, 9 years ago

Resolution: fixed
Status: reopenedclosed

Fixed in 7eeeca2.

comment:11 by Martin Sjölund, 9 years ago

Milestone: 1.9.41.9.4-1.9.x

Milestone renamed

comment:12 by Martin Sjölund, 9 years ago

Milestone: 1.9.4-1.9.x1.9.4

Milestone renamed

Note: See TracTickets for help on using tickets.