New Feature : Definition of more colours for lines in draw mode

Issue #577 resolved
Ronan Le Tiec created an issue

As suggested in the thread http://forum.valentina-project.org/t/new-colors-for-lines/583/ I'd like to implement the use of more colours for the lines in draw mode. The new colours that I'll defined are in the attachment colors.png

Are the new colours ok, or are some of these used for functionalities (like red for instance)?

This will be my first pull request, to make sure I'll do this the right way, is the following procedure right? - make a fork of valentina - perform the changes in the feature branch of the fork - once everything is right and functional, then make a commit with a proper description - finally make the pull request from my feature branch to the develop branch of valentina.

Last question, is it right to do this issue, or should I ask those questions in the forum and there is no need for an issue for this small changes?

Comments (18)

  1. Roman Telezhynskyi repo owner

    Are the new colours ok, or are some of these used for functionalities (like red for instance)?

    Yes, looks like good for me.

    This will be my first pull request, to make sure I'll do this the right way, is the following procedure right?

    Yes. Did you read this page Hacking:Making pull requests?

    Last question, is it right to do this issue, or should I ask those questions in the forum and there is no need for an issue for this small changes?

    Any proposals better first to disscuss in the forum (but this is not true for bugs). But in your case this happend, i did not see any new posts regarding to this thread. So, you did all right.

    If you need any help with the code ask me.

  2. Ronan Le Tiec reporter

    Yes. Did you read this page Hacking:Making pull requests?

    Yes I did, I just wanted to be sure that I understood it well.

    If you need any help with the code ask me.

    All right, thank you, I will.

    Ok, then I'll start right away :-)

  3. Ronan Le Tiec reporter

    Should I add the new colours in v0.3.6.xsd or is it better to create a version 0.3.7 for the pattern format?

  4. Roman Telezhynskyi repo owner

    Better to create new one. I think this is better because in this way we avoid collisions versions.

  5. Ronan Le Tiec reporter

    Ok, I've added the v0.3.7.xsd

    In the feature branch, there is still no v0.3.6.xsd, should I maybe do my changes in the branch "develop" of my fork, to avoid conflicts? For now I have worked on the feature branch on my local code, I added the v0.3.6xsd copying the file from the develop branch.

    Another question, I have defined the color in the ifcdef class and updated the function Colors() and ColorsList() of the vabstracttool. additional to those color-functions, there is the function getPenColor in the vdxfengine.cpp. What is this function used for and should I update this function as well?

  6. Roman Telezhynskyi repo owner

    In the feature branch, there is still no v0.3.6.xsd

    It should be there. Which branch did you use as base for feature branch? Probably you did something wrong.

    What is this function used for and should I update this function as well?

    This is export to DXF format. Yes, if you can find appropriate color in enum DL_Codes::color add it to the list.

  7. Ronan Le Tiec reporter

    It should be there. Which branch did you use as base for feature branch? Probably you did something wrong.

    probably. I forked valentina, cloned the code on my machine, I changed the branch to feature, using "hg update feature", and started do code there. Should I instead create a branch using develop as base and call this branch something different than feature?

  8. Roman Telezhynskyi repo owner

    I see where you made a mistake. After cloning your should change to develop branch. And only then begin work with feature branch.

    By default mercurial set default branch. So you probably made feature branch based on default branch instead of develop.

    Should I instead create a branch using develop as base and call this branch something different than feature?

    No, please, stick to the plan.

  9. Ronan Le Tiec reporter

    I changed back to the develop branch, now everything is fine with the files.

    What i don't understand now, is how to make the feature branch based on the develop. Because there is already a feature branch (the one coming from the repository of valentina) and if I want to create a branch named "feature", then I get the error "abort: a branch of the same name already exists", and the message "use hg update to switch to it". I'm probably missing a step there.

  10. Roman Telezhynskyi repo owner

    The wiki page describes this case:

    1. For regular patches (contain several commits) you should use the branch feature (recommended way). Don't worry if repository already contains one. We can have as many as we need (so called anonymous heads).
    2. See Mercurial useful commands.
    3. hg branch feature - say to commit to feature branch.
    4. hg branch -f feature - say to commit to feature branch even it is already exists. Reopen the branch in another place.
    5. After this all your commits will go to feature branch.
  11. Ronan Le Tiec reporter

    Now everything is commited, including the things you pointed out. I'll take a look at the getPenColor function this week. Thanks for your help!

  12. Roman Telezhynskyi repo owner

    Good, but you could make it quick. The enum is pretty small. Probably you even will not add anything.

  13. Ronan Le Tiec reporter

    I exported a file to dxf, and did a a debug of the color variable from the function VDxfEngine::getPenColor. It looks like it uses only black & white anyway. As I understand, the new colours defined are used only in the Draw-mode, and the dxf export is made from the layout which is black & white.

    So there is probably no need (for the moment) to define more colours here. And you are right, the enum is very small and contains non of the colours that I added.

    Then I will make the pull request now :-)

  14. Log in to comment