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

64232 cleanup scripts #148

Merged
14 commits merged into from Mar 14, 2019
Merged

64232 cleanup scripts #148

14 commits merged into from Mar 14, 2019

Conversation

ghost
Copy link

@ghost ghost commented Mar 12, 2019

Cleanup... is accessible via the markdown extension (Alt + M) and allows the user to update metadata for each file in the working root directory/ repo. Cleanup... does 4 main things:

  • Update .md and .yml files that contain metadata of Array's with a single item, and changes the syntax to use a single item value.
  • Update Microsoft links to use https://, and remove language specific links (en-us etc...)
  • Lowercases the metadata in .md files for certain metadata attributes
  • Allows the user to run all of these scripts at once.

The scripts also touch each file in the repo and replace values. So the total time the scripts take to run/complete will vary based on the size of the repo. Any where from a few seconds to 4 minutes for the sql-docs and azure repos.

Copy link
Contributor

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

Very neat! Are some of these going to be made available through a "Format on Save" type option?

* and cleans up Yaml Metadata values that have single array items
* then converts the array to single item.
*/
function handleSingValuedMetadata() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sing ->Single

return data.replace(regex, `${variable}: ${value}`)
}
catch (error) {
console.error('Error occurred:', error);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure it does, but when the error is logged, does it also log the line number?

}

switch (selection.label.toLowerCase()) {
case "single-valued metadata":
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we using typescript 2? If so we could use the readonly modifier and start moving strings like this to immutable constants.

data = data.replace(technetRegex, "https://technet.microsoft.com")
let docsRegexLang = new RegExp(/https:\/\/docs.microsoft.com\/[A-Za-z]{2}-[A-Za-z]{2}\//g)
data = data.replace(docsRegexLang, "https://docs.microsoft.com/")
let azureRegexLang = new RegExp(/https:\/\/azure.microsoft.com\/[A-Za-z]{2}-[A-Za-z]{2}\//g)
Copy link
Contributor

Choose a reason for hiding this comment

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

The one time i've had to "undo" this is when it was embedded in sample JSON response from the translation API. Probably not worth coding around that though

{
"command": "applyCleanup",
"group": "Docs",
"when": "vscode.extensions.getExtension('docsmsft.docs-article-templates').active"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can change this to "resourceLangId == markdown" so it doesn't have a dependency on the templates extension.

1. Change when clause for applyCleanup command
2. Add js-yaml as dependency
3. Register commands
items.push({
description: "",
label: "$(tasklist) Cleanup...",
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be moved out of the template extension check into the core quickpick menu.

@ghost
Copy link
Author

ghost commented Mar 13, 2019

Very neat! Are some of these going to be made available through a "Format on Save" type option?

Thank you for the suggestion and that is an excellent Idea. I will see if we can make that happen. It would be far less intrusive to keep a markdown linter watching vs running a long process to fix everything all at once.

@nschonni
Copy link
Contributor

@bharney-msft do you want me to file a separate issue?

@ghost
Copy link
Author

ghost commented Mar 13, 2019

@nschonni Sounds good, please do file another issue. I like that idea.

Copy link
Contributor

@jamarw jamarw left a comment

Choose a reason for hiding this comment

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

Looks good!

@ghost ghost merged commit 411d132 into master Mar 14, 2019
@ghost ghost deleted the 64232_CleanupScripts branch March 14, 2019 17:47
This pull request was closed.
Sign up for free to 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.

None yet

4 participants