Fix 611 [Pt 1] #640
Fix 611 [Pt 1] #640
Conversation
A PhantomData was added somewhere in the definition of Document. This caused a lot of lifetimes to be manually specified in various functions prototypes. These lifetimes deliberately always 'static, this will be fixed in next commits. Next commits will focus on adding proper Cow<'a, str> for needed variants. Once this is done, the PhantomData will be removed. Tests have been updated and still pass. This was expectable since there is no change in how the program behaves yet.
All occurences of String in the declaration of Document<'a> have been replaced with Cow<'a, str>. The rest of the code has been updated, tests still pass. There is still a lot of changes to be done. Currently, the Owned variant is always used. Next commits will focus on using the Borrowed variant as much as possible.
Tests in the pretty module always converted their &'static str to Strings, last commits converted it to an Owned variant. This commit changes it to the Borrowed variant, which allows us to remove useless clone.
Previous commit was focused on putting 'static everywhere. This was a mistake here since it would prevent us from using methods avalaible for Document.
This triggered a lot of errors in the codebase. They have all been fixed.
This is useless now, since Document contains other references.
There was a problem hiding this comment.
Thank you for this work, it looks like it has been really fiddly.
I'm not sure I'm happy with what Cow has done to the code here, the amount of duplicated code is extreme and will be quite detrimental to the editing and maintenance experience. Are there combinators that could be used to reduce the duplication?
|
Thanks for the time you took to review my changes. I did my very best and tried to change code as conservatively as possible. I understand your concerns about the huge amount of complexity added by this PR. So far, I have noticed three patterns that could be fixed with the use of a procedural macro. They are:
For these three patterns, I have a solution which I am implementing on a crate I entitled There is also some complexity that can't (as far as I know) be simplified. For example, many lifetimes have to be written manually now. They are not hard to understand, but definitely add some complexity. This list may not be complete. Don't hesitate to report every pattern introduced here that you want to disappear, so I can investigate to find a fix. I'll work on |
The following changes were made:
Documentwhere changed:Stringwas replaced byCow<'a, str>,Documentablefor&str.These two little changes triggered a lot of errors which are now fixed.