basic custom quote support #544
Conversation
Codecov Report
@@ Coverage Diff @@
## master #544 +/- ##
==========================================
+ Coverage 61.28% 61.58% +0.30%
==========================================
Files 32 32
Lines 15639 15837 +198
==========================================
+ Hits 9584 9754 +170
- Misses 6055 6083 +28
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Thanks for PR! Could you please describe your use-case, why you need quotes other than You should run Also note, that each new feature requires a test cases for it, but I think, we could wait with that until you describes rationale for this change. Especially confusing for me why you need different open and close delimiters? |
basic custom quote support remove extra file more consistent styling allow actually using the feature fix logic issue fmt
|
I would like this primarily for parsing |
There was a problem hiding this comment.
Could you add a documentation, when one could use new functions? Maybe mention a your use case, where this could be useful.
You should add tests for all possible inputs that you could imagine, failed cases also need to be tested.
As you introduces a parse state which stores expected open and close delimiters, probably that state could replace SingleQ and DoubleQ states. I would like if you try to do that generalization
| @@ -17,7 +17,9 @@ include = ["src/*", "LICENSE-MIT.md", "README.md"] | |||
| document-features = { version = "0.2", optional = true } | |||
| encoding_rs = { version = "0.8", optional = true } | |||
| serde = { version = "1.0.100", optional = true } | |||
| tokio = { version = "1.0", optional = true, default-features = false, features = ["io-util"] } | |||
| tokio = { version = "1.0", optional = true, default-features = false, features = [ | |||
There was a problem hiding this comment.
Please revert changes in that file, they are not related to the feature
| /// Returns an iterator over the attributes of this tag, with custom quotes. | ||
| pub fn attributes_with_custom_quotes(&self, custom_quotes: &'a [(u8, u8)]) -> Attributes { | ||
| Attributes::wrap(&self.buf, self.name_len, custom_quotes, false) |
There was a problem hiding this comment.
This requires documentation for how you plan to use that method. For what you need an array of custom quotes? How user of this API should fill it? Is it not enough to have one opening and closing quote?
There was a problem hiding this comment.
Ok, I understood the idea. custom_quotes is an array of possible pairs. I propose to use an F: Fn(u8) -> Option<u8> instead, which would map open delimiter to close delimiter or to None if the input is not an open delimiter. Then you should save the gotten close delimiter in the state and do check against it when you checking for a close delimiter.
You don't have to return Attributes here, you can return IterState (or a new wrapper around it if you find wrapper more ergonomic). Thus Attribute struct remains unchanged and you will get a way to get the attribute shape in your code.
| /// The quotes of the attribute value. | ||
| pub quote: Attr<()>, |
There was a problem hiding this comment.
The rationale of this change escapes me. Why it is required? If you want to expose used delimiters, it is better just introduce open: u8 and close: u8 members (or the one tuple member).
|
Anyway, your should also add a point to the |
|
@TheBotlyNoob Do you intend to continue this work? |
|
I don't think so, sorry. I've gotten caught up in other projects. I seem to do that a lot :/. |
No description provided.