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

Pass DocString as String #891

Merged
merged 2 commits into from
Aug 7, 2015
Merged

Pass DocString as String #891

merged 2 commits into from
Aug 7, 2015

Conversation

aslakhellesoy
Copy link
Contributor

With this patch, DocString objects are now true String objects, and not a Cucumber::MultilineArgument::DocString object.

I understand that the intention with Cucumber::MultilineArgument::DocString is that it behaves just like a String, but it doesn't. For example, expect(some_string).to eq(some_doc_string) causes an unreadable error message with a DocString, but a nice one (with a diff) for a String.

Furthermore, DocString is an implementation detail of Cucumber that users shouldn't be concerned with. In DDD speak, these are two (or perhaps three) different bounded contexts: Parsing, (Execution) and user code.

A side-effect of this change is that the content_type attribute is no longer available. That's fine - I can't think of a reason why users would need this. The content_type is purely intended for tools that render Gherkin, and I'm not aware of any tools using it yet.

When those tools exist, the Gherkin3 library should be used to parse the Gherkin document (without Cucumber), and the content_type will be available from the Gherkin3 AST.

Finally, I removed the doc_string method from World - it's not needed, as passing a plain String is sufficient.

@mattwynne
Copy link
Member

Better to deprecate the method rather than remove it so we can release this soon.

I'm curious what motivated you to do this?

@aslakhellesoy
Copy link
Contributor Author

Ok, leave the doc_string method in if you think it'll bite people.

I was working on gherkin-lint, and wasn't happy with the feedback from Cucumber.

@aslakhellesoy aslakhellesoy force-pushed the pass-docstring-as-string branch from 0758e20 to 0463ab6 Compare July 29, 2015 03:59
@aslakhellesoy aslakhellesoy force-pushed the pass-docstring-as-string branch from 0463ab6 to 4e5bd49 Compare July 29, 2015 04:06
@aslakhellesoy
Copy link
Contributor Author

@mattwynne I've kept the doc_string method and deprecated it. Any objections if I merge this to master?

mattwynne added a commit that referenced this pull request Aug 7, 2015
@mattwynne mattwynne merged commit 65f5023 into master Aug 7, 2015
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
@luke-hill luke-hill deleted the pass-docstring-as-string branch March 15, 2019 12:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants