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

Publish v1.8.0 #1495

Closed
bitwiseman opened this issue Aug 22, 2018 · 31 comments
Closed

Publish v1.8.0 #1495

bitwiseman opened this issue Aug 22, 2018 · 31 comments
Milestone

Comments

@bitwiseman
Copy link
Member

bitwiseman commented Aug 22, 2018

@garretwilson @amandabot @HookyQR @astronomersiva
@madman-bob @swan46 @LoganDark @MacKLess
@Mlocik97-issues @Hirse @jdavisclark

At this point the 1.8.0 release includes at least 80 fixed bugs and enhancements. HTML and CSS/LESS/SCSS beautification have both seen massive improvements. Not that performance is primary focus of this project, but the HTML beautifier in particular has seen 10x or more performance improvement.

I'd like to tie off this RC cycle and publish 1.8.0 (release). The latest 1.8.0-rc12 is up on http://jsbeautifier.org/ and npm and pypi.

If you can try out some of the inputs you use most or otherwise give this RC a sanity test drive, I'd be most grateful.

I plan to release soon.

@bitwiseman bitwiseman added this to the v1.8.x milestone Aug 22, 2018
@LoganDark
Copy link

LoganDark commented Aug 22, 2018

LGTM

@garretwilson
Copy link

garretwilson commented Aug 22, 2018

I'm very excited about this. I hadn't been wanting to rush you, but I've been watching the release candidates with anticipation. You know that I'm anxious to get the fix for #1033 published.

I updated atom-beautify to the latest v0.33.0, and then I tried to follow my own directions at #1033 for testing that fix. According to my notes, I copied the entire contents of js/lib in the js-beautify distribution to ~/.atom/packages/atom-beautify/node_modules/js-beautify/js/lib, replacing the atom-beautify files there.

But now there doesn't seem to be a js/lib directory anymore, neither in the latest js-beautify-1.8.0-rc12.zip nor in the commit pull from master (3374af3). Did I miss some step, or has the distribution layout been changed?

@bitwiseman
Copy link
Member Author

bitwiseman commented Aug 22, 2018

@garretwilson

Ah yes, sorry, the files are not checked in on master any more.
They are available from the gh-pages branch - which is where the website is served from
at https://github.com/beautify-web/js-beautify/tree/gh-pages/js/lib but requires a bunch of steps to download them.

But here's a zip of the lib directory. Follow the same instructions as before with this.
lib.zip

I too am exicited to get the the inline element fix out, but also I'm pleased to say there is basic support of optional html tags. So this input remains unchanged in the latest whereas before it would over-indent:

<ul>
    <li>Item 1
    <li>Item 2
    <li>Item 3
</ul>

Also, they can be gotten from here: https://github.com/beautify-web/js-beautify/archive/gh-pages.zip

@garretwilson
Copy link

garretwilson commented Aug 22, 2018

the files are not checked in on master any more.

So I'm unclear then how they will be used by atom-beautify. I'm not an expert on how the Atom plugin system works. Will Atom automatically build the js-beautify dependency and generate these files as part of the Atom plugin installation procedure? Or will @Glavin001 have to do extra work now to get js-beautify integrated?

@bitwiseman
Copy link
Member Author

@garretwilson

In the final release the atom plugin will get the files from the npm package: https://registry.npmjs.org/js-beautify/-/js-beautify-1.8.0-rc12.tgz

There no change for any normal usage. We only stopped checking in the built files in the master branch as they are not interesting and they were was causing confusion for contributors trying to figure out what files they should edit.

@bitwiseman bitwiseman modified the milestones: v1.8.x, 1.8.0 Aug 22, 2018
@Hirse
Copy link
Contributor

Hirse commented Aug 22, 2018

For the Brackets extension I usually grab the files from that folder because I don't need the rest of the package.
Would it be possible to also add them to the release on GitHub?

@astronomersiva
Copy link

Thanks a lot for your continuous efforts on this @bitwiseman!

I ran into a issue while trying out 1.8.0-rc12. I have submitted an issue for this.

@bitwiseman
Copy link
Member Author

@astronomersiva - Thanks! It looks like @MacKLess has submitted a PR with fix.

@Hirse - I'd really rather not redo what is already available from npm.
I've submitted brackets-beautify/brackets-beautify#266 that let's you easily update the files from the npm package (but also has js-beautify as devDependency so it won't get installed on user machines).

@ghost
Copy link

ghost commented Aug 22, 2018

My sanity check passed: the beautifier outputs look good, and I'm still certifiably sane.
I'm super excited about this getting released; well done to all contributors who helped.

@garretwilson
Copy link

garretwilson commented Aug 22, 2018

But here's a zip of the lib directory.

I tested this with the latest atom-beautify v0.33.0 (which is close to the same version I used to verify #1033), and it crashed:

ReferenceError: token is not defined
    at Beautifier.beautify (C:\Users\user\.atom\packages\atom-beautify\node_modules\js-beautify\js\lib\beautify-html.js:1530:29)
    at style_html (C:\Users\user\.atom\packages\atom-beautify\node_modules\js-beautify\js\lib\beautify-html.js:1150:21)
    at exports.html_beautify (C:\Users\user\.atom\packages\atom-beautify\node_modules\js-beautify\js\lib\beautify-html.js:2258:16)
    at file:///C:/Users/user/.atom/packages/atom-beautify/src/beautifiers/js-beautify.coffee:48:20
    at Promise._execute (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\debuggability.js:303:9)
    at Promise._resolveFromExecutor (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\promise.js:483:18)
    at new Promise (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\promise.js:79:10)
    at JSBeautify.module.exports.JSBeautify.beautify (file:///C:/Users/user/.atom/packages/atom-beautify/src/beautifiers/js-beautify.coffee:32:12)
    at file:///C:/Users/user/.atom/packages/atom-beautify/src/beautifiers/index.coffee:361:28
    at tryCatcher (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\util.js:16:23)
    at Promise._settlePromiseFromHandler (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\promise.js:512:31)
    at Promise._settlePromise (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\promise.js:569:18)
    at Promise._settlePromiseCtx (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\promise.js:606:10)
    at Async._drainQueue (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\async.js:138:12)
    at Async._drainQueues (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\async.js:143:10)
    at Async.drainQueues (C:\Users\user\.atom\packages\atom-beautify\node_modules\bluebird\js\release\async.js:17:14)
    at <anonymous>

(This happened in three out of the four files I tried.)

Doesn't look good. Please don't release this.

@bitwiseman
Copy link
Member Author

@amandabot The congrats include you. 😄

@garretwilson
No problem, thanks for trying. There will be an rc13 tomorrow (see #1496 and the fix PR #1499).

@garretwilson
Copy link

There will be an rc13 tomorrow (see #1496 and the fix PR #1499).

PR #1499 really has me worried from glancing at the comments. I sincerely hope the workaround is not to change content by changing a non-breaking space to a normal space. Please do not corrupt the content. Wrap on normal breaking whitespace, and leave everything else as it is.

@bitwiseman
Copy link
Member Author

@garretwilson
rc13 is released.
Download https://github.com/beautify-web/js-beautify/archive/gh-pages.zip again to get the latest files.
@astronomersiva - you're issue should be fixed now.

@garretwilson
Copy link

garretwilson commented Aug 23, 2018

Regarding rc13, I have good news and bad news. The first good news is it doesn't crash anymore. Hooray!

More good news: remember in #1033 how I was overjoyed that inline elements finally were being formatted correctly, but I mentioned that <figcaption> inside a <figure> wasn't getting a line break, if it followed an inline element such as <img/>:

	<figure class="near side"><img src="images/flyway-schema_version-table.png" alt="Flyway schema_version table." /> <figcaption>Flyway schema_version table. (<a href="https://flywaydb.org/getstarted/how"><cite>How Flyway works</cite></a>)</figcaption>
	</figure>

This wasn't a huge issue and I could live with it. Still I'm happy to say that this is now fixed in rc13 (and in rc12 as well, when it didn't crash)!

	<figure class="near side"><img src="images/flyway-schema_version-table.png" alt="Flyway schema_version table." />
		<figcaption>Flyway schema_version table. (<a href="https://flywaydb.org/getstarted/how"><cite>How Flyway works</cite></a>)</figcaption>
	</figure>

It also improves other formatting inside <figure>, such as </code></pre></figure> having a line break before </figure> now like it should.

However there is a problem. There is a pretty big regression for indenting nested lists. The algorithm now seems to get confused about list levels, and list items in the nested lists get indented backwards! I've filed issue #1501.

@garretwilson
Copy link

(Unrelated to HTML, I see that in CSS the rules inside a @media block are now separate by blank lines instead of being forced together vertically. Nice, thanks.)

@bitwiseman
Copy link
Member Author

@garretwilson 1.8.0-rc14 published - fixes #1501.

@garretwilson
Copy link

garretwilson commented Aug 23, 2018

also I'm pleased to say there is basic support of optional html tags.

I was going to make a snide comment when I first saw this. I will never understand why people want to make non-well-formed content. Is it really so difficult to add an ending tag?

The problem is that when you have optional ending tags is that it suddenly makes the content really hard to parse (unless you have a well-designed parser rigorously following a clear grammar, which I'm not sure describes the case here). To resolve ambiguities you have to "guess" at what the person intended, and if guessing what people intend is hard for people, imagine for computers.

So if you want the parser to be flexible to handle the most lazy of authors with the most non-well-formed content, fine. Please be very careful to make sure it doesn't break the content of those of us who are trying our best to make well-formed content that aligns to the spec.

I hope you'll forgive the little soap-box speech; it's an issue I see across the industry.

I'll try to test the fix for #1501 today, but now I worry more about content across the board.

(P.S. Yes, I know that HTML allows this, and yes I know that HTML was never XML compliant. I'm just expressing a general sentiment.)

@MacKLess
Copy link
Collaborator

MacKLess commented Aug 23, 2018 via email

@garretwilson
Copy link

Hey, if it works for both types of authors, great. I just went off on a tangent about things I think about. 😁

@bitwiseman , is there a Zip file handy of the rc14 files for atom-beautify?

@bitwiseman
Copy link
Member Author

@garretwilson
Copy link

garretwilson commented Aug 24, 2018

I'm afraid I won't get to this today. I'm swamped with work and I have to take a few minutes' rest.

But thinking more about this, I'm still a little worried about this new feature of handling non-closed elements. Were a lot of in-depth test cases added to the code to make sure it works correctly in corner cases, and doesn't harm existing content? It worries me a little that bugs like this would be surfacing on the 14th release candidate. If my worry is needless and there are lots of unit tests, etc., then pardon my bringing it up. I haven't looked that the code; you guys would have a better handle on how safe and tested it is.

@garretwilson
Copy link

garretwilson commented Aug 24, 2018

I went ahead and took the time to test this tonight.

This rc14 is still showing bugs in de-indented nested definition lists <dl>/<dt>/<dd>. I've filed ticket #1507.

When so many bugs in the codebase is appearing on the 14th release candidate, it is apparent that the code is not ready for production.

I would request that you remove this new code and provide sufficient unit tests before putting into the release stream. I hope you produce a release candidate without that code, and go ahead and release 1.8.0 without it as well.

I have literally been waiting years for #1033 to be fixed, and after contributing advice and money, I was overjoyed to see that it got addressed. I was hoping you'd just release a version that included that fix, but now it seems like everything is dragging on, waiting for v1.8.0, and even adding new things. Can't we just get the fix for #1033 into the wild?

The problem now is that to get back to the #1033 fix, I'm going to have to go back to the branch that #1033 was fixed on and copy the files from there just so that I can do my day-to-day work.

Sorry for all the complaints. You don't know how I've been biting my tongue ever since #1033 was fixed to keep from bothering you with "Is it released yet? Is it released yet?" questions every day.

OK, I'm pretty tired. I'll check on the status of this tomorrow. Good night.

@garretwilson
Copy link

garretwilson commented Aug 24, 2018

As I noted in #1507, it turns out I was tired and unzipped r13 when testing instead of r14, so the de-indented nested definition lists do not appear in r14. My mistake. I should have waited until tomorrow to test this as I originally intended. But I'm so anxious for it to be released!

Anyway, my overall concerns remain, but as I mentioned, you guys know better than I do how solid the new code is.

@bitwiseman
Copy link
Member Author

@garretwilson
This happens with any software release, you are just getting to see and be part of the process in this case. 😄

I understand how you feel but there have only been two bugs added since I opened this issue. One was in the tricky area of unicode and whitespace handling, and the other was on the new feature of optional closing tags. They were both easily understood and fixed. The problems are specific, not systemic.

We're almost there. Just a day or two more of banging on rc14 and it'll be out the door.

@bitwiseman
Copy link
Member Author

Okay, it's gone quiet here. That's good.

I'm not going to ship a huge release like this on a Friday.

But unless something big comes in, 1.8.0 will go out early on Monday.

@gabrielmaldi
Copy link
Contributor

Great work!

Just reported: #1514 and #781 (comment)

Thanks!

@bitwiseman
Copy link
Member Author

@gabrielmaldi And you couldn't tell me about #1514 before release? 😄
Released 1.8.1.

@garretwilson
Copy link

Is there a Zip I can get of the 1.8.1 files? I'm waiting for @Glavin001 to integrate this into atom-beautify, so I have to keep manually updating my installation files. Thanks.

@Glavin001
Copy link

I'm waiting for @Glavin001 to integrate this into atom-beautify, so I have to keep manually updating my installation files.

@garretwilson : Atom-Beautify's version range for js-beautify is ^1.7.5: https://github.com/Glavin001/atom-beautify/blob/master/package.json#L194

Since the new release is 1.8.1 you can simply uninstall and reinstall Atom-Beautify to trigger an update of these dependencies, such as js-beautify. See https://semver.npmjs.com/ for the semver matching:

image

Even if the above was not possible, I do not see an open Pull Request for updating js-beautify dependency to 1.8.1: https://github.com/Glavin001/atom-beautify/pulls

I am not actively watching for changes to js-beautify or any other dependency. My current priority is the future of Atom-Beautify, which is https://unibeautify.com/ . If there is an update you want to see in Atom-Beautify, I recommend you submit a Pull Request and mention both @szeck87 and I (@szeck87 is much more involved with Atom-Beautify lately, as I've been prioritizing Unibeautify) to get it reviewed and merged. Thanks.

@garretwilson
Copy link

garretwilson commented Aug 31, 2018

@Glavin001 ,

Atom-Beautify's version range for js-beautify is ^1.7.5: … Since the new release is 1.8.1 you can simply uninstall and reinstall Atom-Beautify to trigger an update of these dependencies, such as js-beautify.

Oh, that's nice! Great.

But the problem is that atom-beautify is hard-coding duplicate defaults (which I've cautioned against in the past), and because the fix for js-beautify #1033 requires changing the default value of unformatted, atom-beautify will have to change the defaults as well.

I requested this in Glavin001/atom-beautify#2210 . (If you would simply remove the redundant default settings, delegating to js-beautify's default settings, we wouldn't have to keep doing this every time js-beautify tweaks its default values; that would be my preference.)

@Glavin001
Copy link

I commented at Glavin001/atom-beautify#2210 (comment) . Please continue the Atom-Beautify related discussion there. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants