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

Introduced better fix for pipes escaping in Markdown table cells #1611

Merged
merged 2 commits into from
Oct 4, 2017

Conversation

slavafomin
Copy link
Contributor

I've noticed that table's row in UglifyjsWebpackPlugin documentation was broken:

uglifyjswebpackplugin

I've tried to fix the source file, but it appeared to be correct. After further investigation the problem was related to this issue: markedjs/marked#595.

I've found the piece of code, which was responsible for fixing it in this repo and rewritten it to handle more broad use cases (it was working only for the single pipe in the table cell and was failing for multiple pipes). I've used finite state machine approach to implement table row parser, which fixes this issue.

The result:

localhost-3000-plugins-uglifyjs-webpack-plugin-

I hope the implementation is correct, but please verify it.

Thanks.

@skipjack
Copy link
Collaborator

@slavafomin yeah I think the current fix doesn't handle multiple escaped pipes in a single cell. Thanks for taking the time to research a better approach, I'll pull and test thoroughly as soon as I have some time.

@slavafomin
Copy link
Contributor Author

Is there anything I can do to help with the PR review and acceptance? This is a critical issue and I hope it could be deployed soon.

@skipjack
Copy link
Collaborator

skipjack commented Oct 3, 2017

@slavafomin sorry for the delay. I will test this out tonight, and assuming there's no issues, merge it as well. Thanks again for your work on this.

Copy link
Collaborator

@skipjack skipjack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and confirmed the fix.

@skipjack skipjack merged commit 16a0e82 into webpack:master Oct 4, 2017
dear-lizhihua referenced this pull request in docschina/webpack.js.org Oct 17, 2017
* update /content/loaders & /content/plugins

* docs(api): minor fix in cli

Make minor fix in the CLI documentation to reflect this comment:

webpack#1577 (comment)

* docs(plugins): clarify and simplify `SourceMapDevToolPlugin` docs (#1581)

Try to resolve the confusion from #1556. Also update the options
section to clean things up a bit both for readability and grammar.

Resolves #1556

* docs(guides): update "external limitations" example in author-libraries (#1583)

* docs(plugins): update commons-chunk-plugin.md (#1560)

Changed example to declare the plugin within `plugins` property.

* docs(guides): rewrite shimming (#1579)

Rewrite the main sections of the guide to synchronize it with
the other core guides. Briefly mention the other utilities that
don't require full on examples (details can be found via links).

Closes #1258

* docs(guides): fix small issues in shimming (#1591)

* docs(guides): fix entry path in typescript (#1592)

* docs(guides): fix typos in production (#1584)

* fix(sponsors): update to reflect opencollective change (#1588)

OpenCollective's structure changed a bit. This address that
and makes a few other notable changes:

- add additional sponsors
- allow to merge additional sponsors to existing sponsors
- limit width of logos to 3 x height to disallow very wide logos
- format money values with commas

* docs(config): update configuration-languages (#1590)

Remove local type def from TypeScript snippet. The @types/node type 
definition module already declares `__dirname` type:

https://git.io/v5Dr9

* docs(guides): update hot-module-replacement (#1539)

Add an example to demonstrate using hot module replacement with 
`webpack-dev-server`'s Node.js API instead of within a normal
configuration file.

* docs(guides): update development.md (#1586)

The article was missing references to `clean-webpack-plugin`
in the `webpack.config.js` file and `package.json` files.

* docs(guides): update tree-shaking.md (#1589)

As a conclusion, I thought it would be a good idea to show case 
a high-level explanation of why it's called tree shaking. Once the 
user reads the guide and ends up with a high-level explanation 
the word tree shaking will stick to the user's head.

* docs(guides): update code-splitting (#1585)

Suggest solutions to the problem right away seems to be a 
better approach. Use similar text as was used here:

https://webpack.js.org/api/module-methods/#import-

* docs(plugins): update module-concatenation-plugin (#1565)

Add more information about optimization bailouts. Attempt to transfer 
content from the blog post to the official docs. I want to follow up with 
a PR for better bailout reasons. The hope is that the reasons will match 
those listed in the table.

* docs(api): fix type in compiler.md (#1601)

* docs(config): update output.md (#1541)

Clarify interactions between `libraryTarget` and `library`. The 
interactions between the two and how some of the sections were not
well explained, in some cases were incorrect, misleading or missing
information.

* docs(api): add concurrent usage warning in node.md (#1599)

* docs(guides): update environment-variables (#1549)

Add documentation about setting environment variables in CLI with 
`--env`. Link to webpack _CLI Environment_ section. Add additional 
usage to `--env` CLI docs.

* update /content/loaders & /content/plugins

* Swap ordering of pre/post loaders (#1602)

`pre` loaders run at the beginning, `post` loaders run at the end.

* docs(sponsors): update segment's donations (#1603)

* fix(fetch): fix some issues when parsing remote content (#1604)

The previous regex was a simpler approach to the lead in content
issue in fetched plugin/loader readmes, however it wasn't targeted
enough to correctly remove all lead in content. The new approach
should be a bit more stable but honestly the real fix here would be
so better standardize/enforce the readme template before including
these packages in the documentation site. I also think we can simplify
the template a bit to make the parsing of these readme easier.

Resolves #1600

* docs(guides): fix typo in code-splitting (#1606)

* docs(config): update `inline` recommendation in dev-server (#1540)

Update tip based on discussion in #1540.

* docs(config): update pfx description in dev-server (#1595)

Clarify the difference between using `pfx` via the command line and
in a configuration.

* docs(config): clarify stats option sort and default (#1596)

* docs(config): add before/after hook info to dev-server (#1608)

* docs(guides): fix typo in development (#1609)

* docs(guides): fix typo in production.md (#1610)

* docs(readme): fix typos (#1615)

* fix(markdown): fix issue with multiple pipes in a single cell (#1611)

Introduced better fix for pipes escaping in Markdown table cells.

* docs(guides): update hot-module-replacement (#1614)

Second argument of `server.listen` is a hostname, in this context, 
`'localhost'`. The third argument of `server.listen` is the success 
callback...

https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L552

* docs(guides): fix `onClick` instances in caching (#1613)

* docs(config): fix typo in module.md (#1618)

* docs(guides): fix grammar in getting-started.md (#1619)

* docs(config): add filename to index example (#1620)

* docs(guides): update wds options in hot-module-replacement (#1617)

Define the host property of the options object and set it to 'localhost'. The 
webpack-dev-server CLI defaults the host to 'localhost', but `addDevServerEntrypoints` 
from the webpack-dev-server Node.js API does not set a default and leaves 
it undefined, which will eventually throw "Uncaught SyntaxError: The URL 
'http:/sockjs-node' is invalid" in the browser when following the code example.

webpack/webpack-dev-server#1129

* fix(sponsors): opencollective api changed, add validation (#1622)

* docs(development): update how-to-write-a-loader.md (#1621)

Changes `_` to `.` to have correct code.

* update /content/loaders & /content/plugins

* update contributors

* docs(plugins): add deepChildren to commons-chunk-plugin.md (#1625)

* feat(sponsors): highlight latest sponsors/backers (#1628)

* refactor: update information architecture

This commit contains the following changes:

- Merge support and development into a coherent "Contribute" section.
- Flatten `/api/plugins` in preparation for flattened page `group`ing.
- Sort "Contribute" section and rewrite index page using "Why Support?".
- Simplify and organize antwar config as much as possible.
- Move `title` / `description` fields to the actual components.

BREAKING CHANGE: The route restructuring and content reorg
breaks some routes. Before this is merged, we should make sure
to add redirects for anything broken.

* refactor(navigation): update routes and styling

Simplify routing to reflect antwar and content changes and remove
extraneous links. Simplify styling to first improve the design, but also
to provide a lot more breathing room so less hacks are needed for
smaller page sizes. With the updated design we should have room
for "Voting" and "Analyze" links once those micro apps are in a
better place.

* fix: add link to starter kits

"Support" is now included in the "Contribute" section and already has
a link in the top navigation. The starter-kits page was previously hidden
from users, so this exposes it a bit more.

* refactor(navigation): update the navigation component

Make minor styling change for consistent gutter. Extract link data into
a separate JSON file. Update variable and parameter names for clarity.

* feat: allow page grouping within sections via `group` field

Add support for a new `group` field in each page's YAML frontmatter to
group pages together in an intuitive way. The benefit of this approach vs
directories is that changing a pages group within the same top-level section
won't affect its route and thus will not break links to it.

* style(page): align content better on larger screens

* docs: remove empty pages

* docs(contribute): add writers-guide and update release-process

* docs: sort the base pages more intuitively

* docs(api): update page names and grouping

* fix(mobile): clean up mobile sidebar data

* docs(api): clean up formatting and fix linting errors

* docs(api): fix markdown listing issues

* docs(contribute): fix markdown linting issues

* docs(api): rewrite the index page

* docs(api): minor formatting and cleanup

* docs: add redirects for all broken routes

The changes in #1612 break a significant amount of pre-existing URLs, this
commit adds redirects for everything I could see that would be broken.

* style(page): fix small padding issue

The padding here is useful for browsers that display a full on scrollbar
instead of an overlay scrollbar. We can continue to think about how
that space it utilized in future commits/prs.

* docs(guides): update production.md (#1629)

Modify the doc accordingly to the following tweetstorm where Dan 
Abramov & Sean Larkin kindly explained this to me.

https://twitter.com/TheLarkInn/status/917524723579338752

* fix(markdown): wrap inline code (#1634)

This change makes inline code blocks wrap so long lines will not 
be hidden outside paragraph.

Resolves #1633

* docs(guides): fix small typo excluding closing brace (#1637)

* docs(guides): fix typo in development.md  (#1638)

* docs(guides): update hot-module-replacement.md (#1639)

Maintain consistent code from previous guide (development).

* docs(guides): maintain consistent code in production.md (#1640)

* docs(guides): update typescript.md

Configure TypeScript to compile modules to ES2015 module syntax,
enabling webpack to resolve the `import` / `export` syntax on it's own.
With this included, the only thing necessary to enable _tree shaking_
is the inclusion of the `UglifyJSPlugin` (which I'm not adding to this
article as it is just centered on getting started with TypeScript).

Resolves #1627

* update /content/loaders & /content/plugins

* fix LinkDropdown

* 修复 npm 命令错误导致编译不成功的问题
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants