Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Print empty namespace or module. - #13813

Merged
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file
Sep 2, 2022
Merged

Print empty namespace or module.#13813
vzarytovskii merged 5 commits into
dotnet:mainfrom
nojaf:signature-empty-file

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

Fixes#13757

@nojaf
nojaf marked this pull request as ready for review August 31, 2022 09:07
@nojaf

Copy link
Copy Markdown
ContributorAuthor

@vzarytovskii@dsyme ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there are multiple items in this list?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That is a very unlikely scenario I believe.
You can only have this with two namespaces.
Fair enough, I've added a test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also check for TMDefRec here? Consider these (and maybe add to tests):

modulerecFoobardo()
namespacerecFoobardo()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

modulerecFoobardo()

currently produces

moduleFoobarval``doval@3`` :unit

This probably isn't correct but is outside my scope here.

Comment threadsrc/Compiler/TypedTree/TypedTreeOps.fsi Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There seems to be another edge case which could theoretically happen:

namespaceFoodo()namespaceBarmoduleBaz =do()

In this case it won't be matched and namespace Foo won't be in the signatures.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I can live with that, the goal is to avoid purely empty file generation.
This will give a result because of the nested module and that is good enough for my use case.

@nojaf
nojafforce-pushed the signature-empty-file branch from 5cfa0e3 to 54971c5CompareSeptember 2, 2022 07:01
@0101

0101 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

@nojaf we can have a separate issue to track the remaining inconsistencies if they're too difficult to fix right now. If you can add a docstring to (|EmptyModuleOrNamespaces|_|) then I think we can merge this.

@nojaf

nojaf commented Sep 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks 👍, I see that I've added the docstring to the implementation file instead. I'll address that this afternoon.

0101
0101 approved these changes Sep 2, 2022
@vzarytovskii
vzarytovskii merged commit e06e079 into dotnet:mainSep 2, 2022
@nojaf
nojaf deleted the signature-empty-file branch September 2, 2022 14:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signature file generation does not handle empty assembly info input

3 participants

@nojaf@0101@vzarytovskii