Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow indent to use other options than left and hanging #44

Merged
merged 1 commit into from
Sep 16, 2017

Conversation

jacwright
Copy link
Contributor

@jacwright jacwright commented Sep 16, 2017

This is a breaking change. Existing code using indent will break when padding in the number for left.

If you don't like this breaking change I can submit another PR that just adds a 3rd parameter after hanging for the firstLine attribute. This attribute is the most common after left IMO, and the attribute I need. Book manuscripts indent the first line of each paragraph. I made this bigger change because I felt like 3 arguments, all optional, starts to get messy.

Please advise.

@dolanmiu
Copy link
Owner

I like this actually, it makes more sense

Does this change the API for the user? If so, then it has to be a 3.0.0 release thing, if not, then I could bump the version

@jacwright
Copy link
Contributor Author

jacwright commented Sep 16, 2017

It would have to be a 3.0.0 release. If someone is currently using .indent(1234) then it would no longer work for them.

Do you know when you would release 3.0.0? I may submit another non-breaking PR to allow firstLine for now if it won't be for awhile.

@dolanmiu
Copy link
Owner

I think it will be a little longer because I want to make it so you create a:

var document = new docx.File();

which would be breaking

I say this cos Microsoft Word has an odd way to handle images, by having references to other files. The File can contain the Document, and essentially be the manager for all the references.

Document is just an "xml file"

@dolanmiu
Copy link
Owner

How about creating two methods, one old and one new, and when 3.0.0 comes out, remove the old one?

jacwright added a commit to jacwright/docx that referenced this pull request Sep 16, 2017
This is a backwards compatible fix while a better (but breaking) fix is here dolanmiu#44.
@jacwright
Copy link
Contributor Author

Done, this other PR will get me by until 3.0 is out. Thank you.

This is a breaking change. Existing code using indent will break when padding in the number for left.
@dolanmiu dolanmiu merged commit 3a7f905 into dolanmiu:master Sep 16, 2017
@dolanmiu
Copy link
Owner

Thanks.

Will bump version now

@jacwright
Copy link
Contributor Author

jacwright commented Sep 16, 2017

This was the incompatible version! The other PR #45 is compatible with 2.x. Can you revert?

@dolanmiu
Copy link
Owner

Oh god

@dolanmiu
Copy link
Owner

3.0.0 it is! 😂

@jacwright jacwright deleted the indent-bc branch September 16, 2017 23:16
@dolanmiu
Copy link
Owner

3.0.0 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants