Fix trailing whitespaces #40891

Youssef1313 · 2020-08-15T18:36:03Z

Dotnet-GitSync-Bot · 2020-08-15T18:36:08Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Youssef1313 · 2020-08-15T18:40:40Z

docs/design/coreclr/jit/ryujit-tutorial.md

 #### Notes
 The sample I'm going to walk through implements support for pop count (counting the number of '1' bits in a 64-bit value).
- 
+
 We're going to start by assuming that we have a method with a known signature that implements PopCount.
 Here's the implementation we're going to use. It simply takes the input value, and keeps anding with one, and then shifting right.


This have a visual change, which I think is okay.

before removing the space: The two sentences appeared in the same line.
after removing the space: The two sentences appeared in separate line. I think this is the desired behavior.

docs/design/coreclr/profiling/davbr-blog-archive/Attach.md

.../design/coreclr/profiling/davbr-blog-archive/DoStackSnapshot - Callback CONTEXT Registers.md

docs/design/coreclr/profiling/davbr-blog-archive/DoStackSnapshot - Exception Filters.md

Youssef1313 · 2020-08-15T18:48:15Z

I haven't reviewed all the changes. Just picked some files and pointed to examples of undesired changes, and one example of a possibly desired change.

cc: @jkotas @stephentoub What's the action you recommend to do?

jkotas · 2020-08-15T21:14:28Z

Could you please fixup the deltas where you can easily tell that they are not an improvement?

These types of cleanup tend to come with some damage and it is ok. It is hard to be perfect, but we can at least try to reduce the damage.

jkotas · 2020-08-15T21:14:38Z

Thank you for cleaning this up!

docs/design/coreclr/profiling/davbr-blog-archive/Debugging - Activation.md

docs/design/coreclr/profiling/davbr-blog-archive/Debugging - SOS and IDs.md

Youssef1313 · 2020-08-16T07:24:05Z

docs/design/coreclr/profiling/davbr-blog-archive/DoStackSnapshot - HRESULTs.md

+1) No managed frames on stack
+
 If you call DoStackSnapshot when there are no managed functions on your target thread's stack, you can get E\_FAIL.  For example, if you try to walk the stack of a target thread very early on in its execution, there simply might not be any managed frames there yet.  Or, if you try to walk the stack of the finalizer thread while it's waiting to do work, there will certainly be no managed frames on its stack.  It's also possible that walking a stack with no managed frames on it will yield S\_OK instead of E\_FAIL (e.g., if the target thread is jit-compiling the first managed function to be called on that thread).  Again, your code probably doesn't need to worry about all these cases.  If we call your StackSnapshotCallback for a managed frame, you can trust that frame is there.  If we don't call your StackSnapshotCallback, you can assume there are no managed frames on the stack.


This text shows as if it's outside the list item. The rule MD0029 is capable of detecting such violations. So, if you want to fix that, I think it worth to be in a separate PR to fix all violations reported by that rule.

docs/design/coreclr/profiling/davbr-blog-archive/ELT Hooks - The Basics.md

docs/design/coreclr/profiling/davbr-blog-archive/Profiler stack walking Basics and beyond.md

docs/workflow/testing/mono/testing.md

src/mono/mono/mini/cpu-amd64.md

CoffeeFlux · 2020-08-16T14:02:35Z

Thanks for doing this!

docs/design/coreclr/profiling/davbr-blog-archive/Debugging - SOS and IDs.md

…ature Blob Parser for your Profiler.md Co-authored-by: Youssef Victor <31348972+Youssef1313@users.noreply.github.com>

Youssef1313 · 2020-08-16T18:07:55Z

docs/design/coreclr/profiling/davbr-blog-archive/ELT Hooks - tail calls.md

+Three
+Helper (marked for deferred pop)
+Main


Explicit new lines or code block?

docs/design/coreclr/profiling/davbr-blog-archive/Profiler stack walking Basics and beyond.md

…e review this

Youssef1313 · 2020-08-16T18:45:43Z

...ign/coreclr/profiling/davbr-blog-archive/Sample A Signature Blob Parser for your Profiler.md

+            NotifyTypeSimple()
+        NotifyEndType()
+    NotifyEndParam()
+    _… (more parameter notifications occur here if more parameters exist)_


Now that this became a part of code fence, the underscores won't make it italic.

@jkotas Do you want to delete the underscores?

Suggested change

_… (more parameter notifications occur here if more parameters exist)_

… (more parameter notifications occur here if more parameters exist)

Youssef1313 · 2020-08-16T18:57:44Z

@jkotas I've done more fixes. Can you review again?

I also just noticed that pipelines run with this PR, although it doesn't touch any code files. The YML configuration for pipelines excludes the following:

    - eng/Version.Details.xml
    - .github/*
    - docs/*
    - CODE-OF-CONDUCT.md
    - CONTRIBUTING.md
    - LICENSE.TXT
    - PATENTS.TXT
    - README.md
    - SECURITY.md
    - THIRD-PARTY-NOTICES.TXT

shouldn't it exclude ALL markdown files? Not only the mentioned ones? with something like the following:

    - eng/Version.Details.xml
    - .github/*
    - docs/*
    - LICENSE.TXT
    - PATENTS.TXT
    - THIRD-PARTY-NOTICES.TXT
    - '/**/*.md'

Or are there any code/tests that relies on some md files?

Youssef1313 · 2020-08-16T18:59:27Z

@stephentoub @jkotas I also would still recommend disabling trailing_whitespaces from editorconfig for markdown files, and keep it enabled as it's for C# files as there is an analyzer that checks that. Later, if you enabled GH actions and added a markdownlint with MD009 rule enabled, it would be safe to enable trailing_whitespaces for .md again.

And thanks for being so helpful to the community. 🎉

jkotas · 2020-08-20T00:13:50Z

I also just noticed that pipelines run with this PR, although it doesn't touch any code files.
Or are there any code/tests that relies on some md files?

This can be fixed together with the .md files that are not actually markdown files under mono

jkotas

Thank you!

Youssef1313 and others added 5 commits August 15, 2020 18:26

Create markdownlint.yml

7867d2d

Create markdownlint-problem-matcher.json

90d0411

Create .markdownlint.json

ed2141d

Update .markdownlint.json

72d9853

Trim trailing whitespaces

a4bd240

Delete workflows

29e7d8e

Youssef1313 commented Aug 15, 2020

View reviewed changes

monojenkins mentioned this pull request Aug 15, 2020

Fix trailing whitespaces mono/mono#20254

Merged

Youssef1313 commented Aug 15, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/Attach.md Outdated Show resolved Hide resolved

Youssef1313 commented Aug 15, 2020

View reviewed changes

.../design/coreclr/profiling/davbr-blog-archive/DoStackSnapshot - Callback CONTEXT Registers.md Outdated Show resolved Hide resolved

Youssef1313 commented Aug 15, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/DoStackSnapshot - Exception Filters.md Outdated Show resolved Hide resolved

Youssef1313 marked this pull request as ready for review August 15, 2020 18:46

Youssef1313 requested review from CoffeeFlux, jozkee, lambdageek, layomia, marek-safar, SamMonoRT, steveharter and vargaz as code owners August 15, 2020 18:46

jkotas added the area-Meta label Aug 15, 2020

Youssef1313 commented Aug 16, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/Debugging - Activation.md Outdated Show resolved Hide resolved

Youssef1313 commented Aug 16, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/Debugging - SOS and IDs.md Outdated Show resolved Hide resolved

Youssef1313 commented Aug 16, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/ELT Hooks - The Basics.md Show resolved Hide resolved

Youssef1313 commented Aug 16, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/Profiler stack walking Basics and beyond.md Outdated Show resolved Hide resolved

CoffeeFlux reviewed Aug 16, 2020

View reviewed changes

docs/workflow/testing/mono/testing.md Show resolved Hide resolved

src/mono/mono/mini/cpu-amd64.md Show resolved Hide resolved

jkotas reviewed Aug 16, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/Debugging - SOS and IDs.md Outdated Show resolved Hide resolved

jkotas and others added 3 commits August 16, 2020 09:02

Update docs/design/coreclr/profiling/davbr-blog-archive/Sample A Sign…

ce39223

…ature Blob Parser for your Profiler.md Co-authored-by: Youssef Victor <31348972+Youssef1313@users.noreply.github.com>

Update docs/design/coreclr/profiling/davbr-blog-archive/Sample A Sign…

58ce983

…ature Blob Parser for your Profiler.md Co-authored-by: Youssef Victor <31348972+Youssef1313@users.noreply.github.com>

Manual fixes

9fe64cf

Youssef1313 commented Aug 16, 2020

View reviewed changes

docs/design/coreclr/profiling/davbr-blog-archive/Profiler stack walking Basics and beyond.md Outdated Show resolved Hide resolved

Youssef1313 added 10 commits August 16, 2020 20:15

Needs manual review, the list numbering is really strange

a2ad234

Fix

8954ad4

Code fence

f455538

Deleted extra backslashes and vertical bars, fixed indentation, pleas…

28c8672

…e review this

Make rendered view match the raw md

12e0958

Make rendered view match the raw md

9247682

Match raw with rendered

ae86624

Match raw with rendered

6d38710

Delete extra asterisks and |

7d4f6e9

More cleanup (asterisks and |)

6c22799

Youssef1313 commented Aug 16, 2020

View reviewed changes

Improve readability (review this)

dcda731

jkotas added 3 commits August 19, 2020 17:14

Merge branch 'master' into fix-trailing-whitespaces

2d4c603

Update ELT Hooks - tail calls.md

15ce76c

Update ELT Hooks - tail calls.md

7970628

jkotas approved these changes Aug 20, 2020

View reviewed changes

jkotas merged commit d14b50a into dotnet:master Aug 20, 2020

Youssef1313 deleted the fix-trailing-whitespaces branch August 20, 2020 06:13

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix trailing whitespaces #40891

Fix trailing whitespaces #40891

Youssef1313 commented Aug 15, 2020 •

edited

Loading

Dotnet-GitSync-Bot commented Aug 15, 2020

Youssef1313 Aug 15, 2020

Youssef1313 commented Aug 15, 2020

jkotas commented Aug 15, 2020

jkotas commented Aug 15, 2020

Youssef1313 Aug 16, 2020

CoffeeFlux commented Aug 16, 2020

Youssef1313 Aug 16, 2020

Youssef1313 Aug 16, 2020

Youssef1313 commented Aug 16, 2020

Youssef1313 commented Aug 16, 2020 •

edited

Loading

jkotas commented Aug 20, 2020

jkotas left a comment

		1) No managed frames on stack

		If you call DoStackSnapshot when there are no managed functions on your target thread's stack, you can get E\_FAIL. For example, if you try to walk the stack of a target thread very early on in its execution, there simply might not be any managed frames there yet. Or, if you try to walk the stack of the finalizer thread while it's waiting to do work, there will certainly be no managed frames on its stack. It's also possible that walking a stack with no managed frames on it will yield S\_OK instead of E\_FAIL (e.g., if the target thread is jit-compiling the first managed function to be called on that thread). Again, your code probably doesn't need to worry about all these cases. If we call your StackSnapshotCallback for a managed frame, you can trust that frame is there. If we don't call your StackSnapshotCallback, you can assume there are no managed frames on the stack.

	_… (more parameter notifications occur here if more parameters exist)_
	… (more parameter notifications occur here if more parameters exist)

Fix trailing whitespaces #40891

Fix trailing whitespaces #40891

Conversation

Youssef1313 commented Aug 15, 2020 • edited Loading

Dotnet-GitSync-Bot commented Aug 15, 2020

Youssef1313 Aug 15, 2020

Choose a reason for hiding this comment

Youssef1313 commented Aug 15, 2020

jkotas commented Aug 15, 2020

jkotas commented Aug 15, 2020

Youssef1313 Aug 16, 2020

Choose a reason for hiding this comment

CoffeeFlux commented Aug 16, 2020

Youssef1313 Aug 16, 2020

Choose a reason for hiding this comment

Youssef1313 Aug 16, 2020

Choose a reason for hiding this comment

Youssef1313 commented Aug 16, 2020

Youssef1313 commented Aug 16, 2020 • edited Loading

jkotas commented Aug 20, 2020

jkotas left a comment

Choose a reason for hiding this comment

Youssef1313 commented Aug 15, 2020 •

edited

Loading

Youssef1313 commented Aug 16, 2020 •

edited

Loading