New Feature : Definition of more colours for lines in draw mode
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)
-
repo owner -
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 :-)
-
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?
-
repo owner Better to create new one. I think this is better because in this way we avoid collisions versions.
-
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?
-
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. -
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?
-
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 madefeature
branch based ondefault
branch instead ofdevelop
.Should I instead create a branch using develop as base and call this branch something different than feature?
No, please, stick to the plan.
-
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.
-
repo owner The wiki page describes this case:
- 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).
- See Mercurial useful commands.
hg branch feature
- say to commit to feature branch.hg branch -f feature
- say to commit to feature branch even it is already exists. Reopen the branch in another place.- After this all your commits will go to
feature
branch.
-
reporter Thank you Roman, I had overseen the 4.
I have performed my first commit : https://bitbucket.org/ronanletiec/valentina/commits/03b7a6e85682dccd808ea49ce43719b0442fd51c
I haven't done anything to the VDxfEngine::getPenColor() function yet.
-
repo owner Thank you Roman, I had overseen the 4.
No problem, you are learning fast.
-
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!
-
repo owner Good, but you could make it quick. The enum is pretty small. Probably you even will not add anything.
-
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 :-)
-
repo owner Yes.
-
repo owner - changed status to resolved
Merged in ronanletiec/valentina/feature (pull request
#144)Resolved issue
#577. Definition of 10 new line colors.→ <<cset b89eec981cab>>
-
repo owner - changed status to resolved
Merged in ronanletiec/valentina/feature (pull request
#144)Resolved issue
#577. Definition of 10 new line colors.→ <<cset b89eec981cab>>
- Log in to comment
Yes, looks like good for me.
Yes. Did you read this page Hacking:Making pull requests?
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.