Fix #1065

[acchou]

Fix #1065. Escapes '|' in tables by converting code spans into …, and HTML escape the characters inside.

[msftclas]

All CLA requirements met.

[acchou]

Two questions:

(1) Should I try using rush change as suggested to fix the build error?
(2) Should I add tests for this fix?

[iclanton]

(1) Should I try using rush change as suggested to fix the build error?

Yeah. It'll have you fill out a simple form for your change. Just pick "patch," describe your fix, and then commit the file it drops.

(2) Should I add tests for this fix?

If you wouldn't mind. It'd be good to exercise this with some edge cases. This would probably be a good use of Jest snapshots.

[acchou]

Hmm, rush change gives me this:

Andys-MacBook-Air:api-documenter achou$ rush change
Found configuration in /Users/achou/Code/web-build-tools/rush.json


Rush Multi-Project Build Tool 5.6.1 - https://rushjs.io

Found configuration in /Users/achou/Code/web-build-tools/rush.json

Starting "rush change"

Unable to find a git remote matching the repository URL (undefined). Detected changes are likely to be incorrect.
Target branch is origin/master
No changes were detected on this branch. Nothing to do.

[iclanton]

Question: Do you have a remote set up for the Microsoft fork of web-build-tools? If you don't, change detection may not work correctly. Try adding one with git remote add Microsoft https:/Microsoft/web-build-tools.git and then running git fetch --all. This will add a remote called "Microsoft" on your local clone pointing to this repository and then hydrate it.

It looks like you committed your changes to your fork's master branch, and it's unable to detect the correct remote branch (because you probably don't have the Microsoft/web-build-tools remote set up). Rush is trying to diff your current branch (master) against origin/master (it falls back to origin/master if it can't detect the expected remote), and because your current local branch is tracking origin/master (and you've pushed your changes), there are no differences. Does that make sense? This isn't a scenario we handle particularly well because I think all of the Rush developers are used to working on feature branches :). This is probably something that we should improve the documentation and/or the error messages for.

Edit: However, Unable to find a git remote matching the repository URL (undefined). is an issue. The repository URL shouldn't be printed as undefined because it's configured in rush.json (I checked - your fork is recent enough to also have it configured). That may just be an issue with the error message, or it might indicate a larger issue. Can you try adding the Microsoft remote (outlined above) and running rush change again? If it still fails, there's probably another issue. We recently changed this code and may have inadvertently introduced a regression.

[acchou]

Thanks for the input; I didn't realize I should be working on a branch of my fork or needed to add the remote manually. In any case I added the remote, fixed some tslint issues, added tests and checked in everything. I think I did everything required at this point but not 100% sure so a review would be welcome.

[octogonz]

It looks like you committed your changes to your fork's master branch, and it's unable to detect the correct remote branch (because you probably don't have the Microsoft/web-build-tools remote set up). Rush is trying to diff your current branch (master) against origin/master (it falls back to origin/master if it can't detect the expected remote), and because your current local branch is tracking origin/master (and you've pushed your changes), there are no differences. Does that make sense? This isn't a scenario we handle particularly well because I think all of the Rush developers are used to working on feature branches :). This is probably something that we should improve the documentation and/or the error messages for.

Edit: However, Unable to find a git remote matching the repository URL (undefined). is an issue. The repository URL shouldn't be printed as undefined because it's configured in rush.json (I checked - your fork is recent enough to also have it configured). That may just be an issue with the error message, or it might indicate a larger issue. Can you try adding the Microsoft remote (outlined above) and running rush change again? If it still fails, there's probably another issue. We recently changed this code and may have inadvertently introduced a regression.

@iclanton if we can repro this issue, we should open a GitHub issue and get it fixed (along with #1177). It's fairly common for people to commit directly to origin/master in a forked repo, so rush change should support that if possible.

[iclanton]

@octogonz - agreed. We should investigate this further.

@acchou - I'll take a look this evening.

[iclanton]

Fix #1065: rendering of type unions in tables. [](start = 18, length = 46)

Can you update this to "Fix an issue with type unions rendering in markdown tables (addresses #1065)"

[acchou]

Ok. I just pushed an update with this change.

[iclanton]

🕐

[iclanton]