Comments (23)
Oh wow, I usually catch large diffs like that. I must have had ignore whitespace checked on my diff. It's probably a line ending issue since it appears on lines with no tabbing too. I'll fix that tomorrow or this week end. Thanks for catching that!
from premailer.net.
I think I found the root of the issue. It seems that the line endings of the files in the git index are inconsistent. My git configuration is set to checkout as clrf locally and commit as lf which is the default for a windows git installation.
Note: /i stands for index, /w stands for working directory
PS C:\projects\PreMailer.Net\PreMailer.Net> git ls-files --eol
i/lf w/crlf attr/ .editorconfig
i/lf w/crlf attr/ .gitignore
i/lf w/crlf attr/ Benchmarks/Benchmarks.csproj
i/lf w/crlf attr/ Benchmarks/Program.cs
i/crlf w/crlf attr/ PreMailer.Net.Tests/CssAttributeTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/CssElementStyleResolverTests.cs
i/crlf w/crlf attr/ PreMailer.Net.Tests/CssParserTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/CssSelectorParserTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/CssSelectorTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/CssSpecificityTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/CssStyleEquivalenceTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/Extensions/NodeExtensionTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/LinkTagCssSourceTests.cs
i/crlf w/crlf attr/ PreMailer.Net.Tests/PreMailer.Net.Tests.csproj
i/lf w/crlf attr/ PreMailer.Net.Tests/PreMailerTests.cs
i/lf w/crlf attr/ PreMailer.Net.Tests/StyleClassApplierTests.cs
i/crlf w/crlf attr/ PreMailer.Net.Tests/StyleClassTests.cs
i/lf w/crlf attr/ PreMailer.Net.sln
i/lf w/crlf attr/ PreMailer.Net/AttributeToCss.cs
i/crlf w/crlf attr/ PreMailer.Net/CssAttribute.cs
i/lf w/crlf attr/ PreMailer.Net/CssAttributeCollection.cs
i/lf w/crlf attr/ PreMailer.Net/CssElementStyleResolver.cs
i/crlf w/crlf attr/ PreMailer.Net/CssParser.cs
i/lf w/crlf attr/ PreMailer.Net/CssSelector.cs
i/lf w/crlf attr/ PreMailer.Net/CssSelectorParser.cs
i/lf w/crlf attr/ PreMailer.Net/CssSpecificity.cs
i/lf w/crlf attr/ PreMailer.Net/CssStyleEquivalence.cs
i/lf w/crlf attr/ PreMailer.Net/Downloaders/IWebDownloader.cs
i/lf w/crlf attr/ PreMailer.Net/Downloaders/WebDownloader.cs
i/lf w/crlf attr/ PreMailer.Net/Extensions/NodeExtensions.cs
i/lf w/crlf attr/ PreMailer.Net/ICssSelectorParser.cs
i/lf w/crlf attr/ PreMailer.Net/InlineResult.cs
i/crlf w/crlf attr/ PreMailer.Net/PreMailer.Net.csproj
i/-text w/-text attr/ PreMailer.Net/PreMailer.Net.snk
i/crlf w/crlf attr/ PreMailer.Net/PreMailer.cs
i/lf w/crlf attr/ PreMailer.Net/Sources/DocumentStyleTagCssSource.cs
i/lf w/crlf attr/ PreMailer.Net/Sources/ICssSource.cs
i/lf w/crlf attr/ PreMailer.Net/Sources/LinkTagCssSource.cs
i/lf w/crlf attr/ PreMailer.Net/Sources/StringCssSource.cs
i/crlf w/crlf attr/ PreMailer.Net/StyleClass.cs
i/lf w/crlf attr/ PreMailer.Net/StyleClassApplier.cs
i/lf w/crlf attr/ nuget.bat
This was the Premailer.cs file before my commit:
i/crlf w/crlf attr/ PreMailer.Net/PreMailer.cs
And this is after my commit:
i/lf w/crlf attr/ PreMailer.Net/PreMailer.cs
Since my git installation commits as lf, it converted the file and marked every line as changed.
I recommend that we normalize the line endings in the repository to prevent this issue from creeping up for other contributors as well. I can make a commit using the git command : git add --renormalize .
to normalize the git index to lf.
A made a commit here to normalize line endings.
Let me know what you think.
from premailer.net.
Awesome, seems like the right thing to do!
from premailer.net.
@martinnormark That's what I could find from my research so far. I'll play around with publishing a dummy package from a dummy project to make sure things work right and then I'll just copy the cli command over into the action.
from premailer.net.
Hey @jasekiw - thanks for reaching out, and yes I am open to adding maintainers. If you're interested I can add you right away?
I see you're right about a release is pending, I do recall working on some fixes but then everything else took over. I'm not sure if I needed a few fixes included before cutting a new release, but test-wise the library is pretty solid so as they're all passing it should be fine to cut a release.
from premailer.net.
I am definitely interested. This library doesn't seem to be a huge time sink so I can definitely help keep the library updated.
When you say you recall working on some fixes are you perhaps referring to this pr? #348.
I see you have a github action setup for release to nuget so I assume creating a release in github automatically publishes to nuget?
from premailer.net.
Added you as maintainer now. Thanks for reaching out, let me know if you have any questions!
I do recall starting #348 but also #328 seems to be the un-released stuff.
Correct, there's a github action flow that pushes to nuget when tagging a new release here. Only manual thing needed is to update the version and release notes in the project file.
from premailer.net.
I'll start familiarizing myself with the codebase and the unfinished work over the week end. I want to double check everything before making a release and potentially messing something up on my first try. I could look at doing a prerelease be extra careful. I noticed the Draft new release button didn't show on the releases page. I assume you still want to create the release yourself after I let you know a release is ready?
from premailer.net.
It seems that your invite is still pending, did you click "accept" or similar in an email from GtiHub?
from premailer.net.
My apologies, that was it. The repository appeared to show more access but I was mistaken since I hadn't accepted the invite yet. What is your opinion of me doing a prerelease to test waters?
from premailer.net.
Cool, good to hear. Give it a go man, feel at home!
from premailer.net.
I noticed that all of the classes are marked as public. If someone uses the library in a way the readme doesn't state I could accidentally introduce a breaking change for them. This might be too early of a question but how much of library should I assume public api?
from premailer.net.
I can't merge PR's created by me since it requires one approved review but I can't review my own PR. I was able to merge some dependabot PR's however.
from premailer.net.
I can't merge PR's created by me since it requires one approved review but I can't review my own PR. I was able to merge some dependabot PR's however.
Darn, I thought it was possible to avoid reviews for maintainers. I have removed the requirement of a review, since only maintainers can merge PRs it will not be all up for grabs anyway and the review feature is still available.
This might be too early of a question but how much of library should I assume public api?
I see how this could cause a breaking change. I think the main PreMailer
class and the interfaces you'd need to customize functionality could be the only public classes. Changing them should be released only in a major version.
from premailer.net.
Thanks for making that adjustment!
I have been going through the issues and closed ones I confirmed are already completed.
I did notice this issue that was not marked as completed but did have a PR that was merged. However I found the next commit reverted it. It seems like an accident.
If there are not reasons against, I'll re-add this functionality to the library but under a configuration flag since some people may want to receive these as errors.
from premailer.net.
I can't recall why that should be reverted, so I agree that it seems like an accident. Would be great to add back in - nice catch, you're on a roll already!
I noticed some possible formatting settings could be out of sync, based on this commit - I thought .editorconfig
would prevent that? (entire file being changed)
from premailer.net.
@jasekiw I was excited when I saw this, I was hoping for an update soon. This is a great project.
from premailer.net.
@jasekiw I was excited when I saw this, I was hoping for an update soon. This is a great project.
Hi @TopCoder02 I should have some time this week to try and create a new update. Is there anything specific you were looking for?
from premailer.net.
@jasekiw Not at the moment, the only thing I think would be useful is to remove the styles that are inherited down and remove the browser defaults from being inlined. I'm doing after I use pre-mailer. I'm probably doing it wrong too.
from premailer.net.
@TopCoder02 A new release has been created see https://github.com/milkshakesoftware/PreMailer.Net/releases/tag/v2.5.0
No browser defaults should be inline from what I am aware of, can you create an issue with a reproduction of what you are referring to?
from premailer.net.
@martinnormark I cut a new release. You can see it here: https://github.com/milkshakesoftware/PreMailer.Net/releases/tag/v2.5.0
I did run into one issue however. The action published the new version but then failed on a subsequent line of code. So it worked but is marked as a failure. https://github.com/milkshakesoftware/PreMailer.Net/actions/runs/6983746731
It appears the action hasn't been maintained in some time and I should probably think about an alternative.
from premailer.net.
@jasekiw Awesome with the new version! You rock!
Those actions for pushing a nuget package all seems to be more or less abandoned, I presume since it is quite easy to do with the dotnet cli these days?
from premailer.net.
@TopCoder02 A new release has been created see https://github.com/milkshakesoftware/PreMailer.Net/releases/tag/v2.5.0
No browser defaults should be inline from what I am aware of, can you create an issue with a reproduction of what you are referring to?
I was referring to https://www.w3schools.com/cssref/css_default_values.php
If someone has the style
<style>
h1 {
display: block;
font-size: 2em;
margin-top: 0.67em;
margin-bottom: 0.67em;
margin-left: 0;
margin-right: 0;
font-weight: bold;
}
</style>
Since these values are already the default value, it shouldn't be inlined on an h1 tag, even if it's part of a CSS Class. Since it's already defaulted. I know it's more complicated than this, but that's what I was thinking, it will make the CSS smaller and email size smaller. Because it's more intelligent in inlining, removing unnecessary styles.
from premailer.net.
Related Issues (20)
- MoveCssInline returns self-closing title when empty HOT 5
- MoveCssInline encodes non-ASCII characters even when they should be valid HTML
- Regex in CssParser never ends and uses 100% cpu. HOT 1
- url embedded resources HOT 2
- Request to change colors defined with hsl/hsla to rgb/rgba HOT 1
- Anchor tags should be appended after analytics tags
- Performance issue when process email containing useless big Style tag to small body content HOT 1
- MoveCssInline is HTML encoding CSS HOT 2
- Inliner strips out vml from html tag HOT 1
- Configure continuous benchmark
- Add more example HTML emails to benchmark
- Automate contributors list in README
- Resolve css var's with their actual values?
- Ability to specify which classes should not be removed, like "preheader"
- How to keep html entities (such as ©) from being converted to special characters? HOT 3
- RobiniaDocs API Explorer
- Bug in comment regex, fix in issue description HOT 1
- Two same CSS properties on same dom element does not work HOT 3
- Bump the AngleSharp Version to 1.1.0 HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from premailer.net.