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

Some more cleanups and making the content-type override work the correct way #112

Closed
wants to merge 5 commits into
base: saxon94
from

Conversation

Projects
None yet
2 participants
@Vampire
Copy link

Vampire commented Aug 12, 2013

No description provided.

@ndw

This comment has been minimized.

Copy link
Owner

ndw commented Oct 23, 2013

I'm just not sure about this. Remind me again what you're trying to accomplish?

@ndw ndw closed this Aug 26, 2014

@Vampire

This comment has been minimized.

Copy link
Author

Vampire commented Sep 12, 2014

Oh sorry, forgot to comment on this one.
Would be great if you could reopen it.

9e06163: removes files that are left-overs from some un-clean patch applying
1ca33e1: replaces some "new URI()" with "URI.create()" for known well URIs as you then can save the try/catch and it is the recommended way
fcaf0ea: cleans up some superfluous imports
bc8d007: uses a relative path instead of an absolute path when generating the test report so that it works for people other than you

and most important
257a0e0: In my previous PR you excluded a part that made a unit test fail. This commit does it the correct way so that also the unit test succeeds. Currently the code does not behave as the documentation states, this commit fixes it and brings them together again. It allows the user to override the data input content type from the command-line or the Ant task explicitly without thereby breaking the tests and violating the specification as my first approach did.

@ndw ndw reopened this Sep 13, 2014

@ndw

This comment has been minimized.

Copy link
Owner

ndw commented Sep 13, 2014

Sorry, I'm just having trouble getting my head around the content type patch. By my first reading, you've introduced forced-content-type that basically shadows content-type and maybe gets used preferentially.

What's the use case where content-type doesn't do the right thing, but forced-content-type does?

If you're reading a file: URI, I'd expect content-type to do the right thing. If you're reading from an http: URI but you want to override the server-provided content type, I'd like to understand why.

@Vampire

This comment has been minimized.

Copy link
Author

Vampire commented Sep 13, 2014

I had a use-case in mind when I implemented it, I'm just not 100% sure what it was.
But basically you give the user the possibility to define the content type.

Afair, one usecase is to force the content being treated as text or force the content to being encoded with base64, if you simply manually know better than the detection or if the webserver sends a wrong content type which could pretty well happen out there.

The patch just enables you to specify from command line as parameter or from the ant task the content type to use which is transported as forced-content-type.

@ndw

This comment has been minimized.

Copy link
Owner

ndw commented Mar 6, 2019

Apologies. I didn't get this merged in a reasonable time and there's been too much divergence now. If you've been keeping your patch up-to-date feel free to send it again and I'll try to get my head around it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.