Preserve node marks when cloning a node

[vvd170501]

Solves #812

[vvd170501]

@jbeder, could you review this, please?

[vvd170501]

@jbeder, ping?

[SGSSGene]

@vvd170501 : could you rebase this PR?

I believe this is a nice addition!

[SGSSGene]

Under which circumstance do I not want to preserve the markings?
To me, the additional argument of setting it to false, seems not worth it. What are your thoughts on this?

[vvd170501]

Under which circumstance do I not want to preserve the markings? To me, the additional argument of setting it to false, seems not worth it. What are your thoughts on this?

I don't really have any use cases for not preserving the markings, but this was the original behavior, and someone might have relied on it.

I don't know if yaml-cpp uses semantic versioning, and even if it does, versions are still below 1.0, so breaking changes might be acceptable, but I chose to keep the default behavior just to be safe.

[SGSSGene]

If I would decide, I would leave the preserveMarking option away. I do not see the use case for setting it to false.
It reduces the code change a lot and I think less complexity is better.
But before you change anything, lets see what @jbeder says about this.

@jbeder : Should Clone(const Node& node, bool preserveMarks = false) have a preserveMarks arguments set to default false, which keeps backwards compatibility or should it be removed and assumed to be true to reduce code complexity?

[jbeder]

Let's go with simpler: no preserveMarks arg. Let's make sure to make a note about this (@SGSSGene I've vaguely tried and horribly failed to keep a running list of "breaking changes since last release" or even "notable changes since last release". I wonder if maybe it would be possible to use hashtags on commits, e.g., #breaking: "what changed" or #change: "what's changed" or something, which could then be rolled up at release time. What do you think?)

[SGSSGene]

Let's go with simpler: no preserveMarks arg. Let's make sure to make a note about this (@SGSSGene I've vaguely tried and horribly failed to keep a running list of "breaking changes since last release" or even "notable changes since last release". I wonder if maybe it would be possible to use hashtags on commits, e.g., #breaking: "what changed" or #change: "what's changed" or something, which could then be rolled up at release time. What do you think?)

I was already heavily impressed and wondered how you kept track of the changes for 0.9.0 😅
From now on I will add a '#change' or '#breaking' to the merge message when merging a PR! (I will also check if I can include some information about the PRs already included after 0.9.0)

[vvd170501]

@SGSSGene, @jbeder: I've removed the option, could you launch the build workflow?

[SGSSGene]

I was already heavily impressed and wondered how you kept track of the changes for 0.9.0 😅 From now on I will add a '#change' or '#breaking' to the merge message when merging a PR! (I will also check if I can include some information about the PRs already included after 0.9.0)

@jbeder :
I noticed you deactivated the Allow merge commits and only activated Allow squash merging which allows to add a message but I don't want to squash the commits, or Allow rebase merging which doesn't allow me to set any custom commit message. I would prefer to do Allow merge commits which would allow me to add a message like:
#change: nodes cloning also clones the mark information (line/column)
without losing the commit history.
Could you activate Allow merge commits?

[jbeder]

I'm a big fan of pure linear history, even if it involves rewriting PR commits. So I would just squash and edit the commit message to add the tag.

[jbeder]

In fact, feel free to edit commit messages to make them clearer in general.

[SGSSGene]

I'm a big fan of pure linear history, even if it involves rewriting PR commits. So I would just squash and edit the commit message to add the tag.

By linear commit history, you mean no hit merges, but squash and rebase, so each PR is most of the times a single commit?

I have to check later if I can edit the commits of PRs for this project.

[jbeder]

Yep