Obsidian Plugin Guidelines Conformity and Slice of Life Changes

[pjkaufman]

It seems that there are a lot of guidelines I have missed as they have been added as well as slice of life changes that can be made to the plugin to make it more user friendly. This is a list of things that I think are reasonable to add beyond the normal checklist.

Slice of Life Changes

• Hide extra settings for disabled rules
• Hide general settings that are dependent on other general or rule settings being enabled (Moved setting to the corresponding setting it depends on)
• onLayoutReady let users know if they have rules that conflict via a popup modal that can be disabled via the settings (relates to Bug: File Name Heading inserted numerous times #911)
• onLayoutReady load default list of common auto-correct mappings (see Move Default Auto-Correct Misspellings to Separate File #1185)
  • Maybe even have ability to disable a rule or subset of rules when they are incompatible and force the user to disable one of the incompatible rules/options if it is already selected

Wording Updates

There is a set of guidelines for how wording should go in the UI for plugins, themes, etc. to make sure things are consistent. I need to take a look and see what I should change in the Linter to be consistent with the guidelines located here.

Guidelines

Releasing & naming

• Remove placeholder names such as MyPlugin and SampleSettingTab.
• Don't include the word "Obsidian" in your name unless it absolutely makes sense. Most of the time it's redundant.
• Don't include your plugin name in command names. Obsidian adds this for you.
• Don't prefix commands with your plugin ID. Obsidian adds this for you.
• Don't include main.js in your repo. Only include it in your releases.
• If you haven't, consider add a fundingUrl so that users of your plugin can show some support. Learn more.

Compatibility

• Don't provide default hotkeys for commands. Learn more.

There is an instance of a default hotkey used in the Linter here. I believe it can be removed as I have never really used it and most users probably do not use it anyways.

• Don't override core styling. If needed, add your own class and make the styling only apply to your class.
• Do scan your code for deprecated methods (they usually show up as strikeout text in IDEs).
• Don't assign styles via JavaScript or in HTML. Learn more.

This happens at several places in the code. It has mostly been removed over time, but there are still a couple of places where it exists:

• centering and bolding some elements in modals:

  obsidian-linter/src/ui/modals/lint-confirmation-modal.ts

  Lines 11 to 14 in b1cb567

  	this.contentEl.createEl('h3', {text: getTextInLanguage('warning-text')}).style.textAlign = 'center';
  	if (showCustomCommandWarning) {
  	this.contentEl.createEl('p', {text: getTextInLanguage('custom-command-warning')}).style.fontWeight = 'bold';

  and

  obsidian-linter/src/ui/modals/parse-results-modal.ts

  Lines 11 to 28 in b1cb567

  	this.contentEl.createEl('h3', {text: getTextInLanguage('parse-results-heading-text')}).style.textAlign = 'center';
  	if (customAutoCorrectionInfo?.customReplacements.size > 0) {
  	this.contentEl.createEl('p', {text: getTextInLanguage('file-parse-description-text').replace('{FILE}', customAutoCorrectionInfo.filePath)}).id = 'confirm-dialog';
  	const tableEl = this.contentEl.createDiv().createEl('table', {cls: 'markdown-rendered'});
  	const tableHeaderEl = tableEl.createEl('tr');
  	tableHeaderEl.createEl('th', {text: getTextInLanguage('find-header-text')});
  	tableHeaderEl.createEl('th', {text: getTextInLanguage('replace-header-text')});
  	let tableRow: HTMLElement;
  	for (const replacementInfo of customAutoCorrectionInfo.customReplacements) {
  	tableRow = tableEl.createEl('tr');
  	tableRow.createEl('td', {text: replacementInfo[0]});
  	tableRow.createEl('td', {text: replacementInfo[1]});
  	}
  	} else {
  	this.contentEl.createEl('p', {text: getTextInLanguage('no-parsed-values-found-text').replaceAll('{FILE}', customAutoCorrectionInfo.filePath)}).style.fontWeight = 'bold';

• Centering the settings zero state:

  obsidian-linter/src/ui/settings.ts

  Line 85 in b1cb567

  this.searchZeroState.createEl(isMobile ? 'h3' : 'h2', {text: getTextInLanguage('empty-search-results-text')}).style.textAlign = 'center';

• Removing a border for the search input for the settings:

  obsidian-linter/src/ui/linter-components/tab-components/tab-searcher.ts

  Line 24 in b1cb567

  searchSetting.settingEl.style.border = 'none';

• Don't access the hardcoded .obsidian folder if you need to access the configuration directory. The location could be customized, so please use Vault.configDir instead.

Mobile support

Please only complete this section if you have isDesktopOnly set to false in your manifest.

• Don't use node.js modules such as fs, path, and electron.
• Don't use regex lookbehinds if you want to support iOS versions lower than 16.4 (ignore this if you don't use regex in your plugin). Learn more.
• Don't use the FileSystemAdapter class.
• Don't use process.platform, use Obsidian's Platform instead. Link to API.
• Don't use fetch or axios.get, use Obsidian's requestUrl instead. Link to API.

Coding style

• Don't use var. Use let or const instead. Learn more.
• Don't use the global app instance. Use this.app provided to your plugin instance instead. Learn more.
• Do break up your main.ts into smaller files or even folders if it gets big to make code easier to find.
• Do use async and await when you can for readability, instead of using Promise. Learn more.

This is a skill issue of mine. I am not really a JS dev normally, so Promises and async and await make some sense, but a lot goes over my head.

Places that expressly say Promise and use an operator:

• Linting all files in a vault:

  obsidian-linter/src/main.ts

  Line 460 in b1cb567

  await Promise.all(app.vault.getMarkdownFiles().map(async (file) => {

• Linting all files in a folder:

  obsidian-linter/src/main.ts

  Line 486 in b1cb567

  await Promise.all(filesInFolder.map(async (file) => {

• Don't use global variables. Try to keep variables either in the scope of classes or functions. Learn more.

I do this sparingly and do not plan to change unless someone points out a benefit to not doing so where I currently do use them.

• Do test with instanceof before casting into other types such as TFile, TFolder, or FileSystemAdapter,
• Don't use use as any and use proper typing instead.

This is definitely a skill issue on my part just like using @ts-ignore however I don't think I am able to fix all of the places where I use the type any in the code:

• I am not sure how you define a YAML object any other way:

  obsidian-linter/src/rules/yaml-key-sort.ts

  Line 81 in b1cb567

  getYAMLKeysSorted(yaml: string, keys: string[], yamlObject: any): {remainingYaml: string, sortedYamlKeyValues: string} {

• When I use just about any other type, I get TypeScript type errors:

  obsidian-linter/__integration__/main.test.ts

  Line 28 in b1cb567

  private timeoutId: any = undefined;

• Not sure why the param has type any, but I believe it has to do with the underlying type that is expected:

  obsidian-linter/src/ui/components/text-box-full.ts

  Line 56 in b1cb567

  }, (reason: any) => {

• Not sure how you do this any other way:

  obsidian-linter/src/option.ts

  Line 22 in b1cb567

  constructor(public configKey: string, public nameKey: LanguageStringKey, public descriptionKey: LanguageStringKey, public defaultValue: any, ruleAlias?: string | null) {

• Another YAML object instance:

  obsidian-linter/src/utils/yaml.ts

  Line 89 in b1cb567

  export function loadYAML(yaml_text: string): any {

• I am not sure how you get around this here:

  obsidian-linter/src/translation-helper.ts

  Line 152 in b1cb567

  let object = localeMap[language] as {[k: string]: any};

• I could type out the options, but I do not use them if I recall and I am lazy:

  obsidian-linter/src/typings/moment-parseformat.d.ts

  Line 3 in b1cb567

  export = parseFormat;

• I don't think I still need this typing extension:

  obsidian-linter/src/typings/obsidian-ex.d.ts

  Line 56 in b1cb567

  getState(): any;

• I am not sure how I avoid either of these:

  obsidian-linter/src/rules.ts

  Line 15 in b1cb567

  export type Options = { [optionName: string]: any};

  and

  obsidian-linter/src/rules.ts

  Line 66 in b1cb567

  const options: { [optionName: string]: any } = {};

API usage

• Don't use Vault.modify. If you want to edit the active file, prefer using the Editor interface. If you want to edit it in the background, use Vault.process.

This is probably a simple swapping out of the functions used and testing that it works:

obsidian-linter/src/main.ts

Line 437 in b1cb567

await this.app.vault.modify(file, newText);

• Don't manually read and write frontmatter. Instead, use FileManager.processFrontMatter. Learn more.

This ain't going to change since the Linter has to do so to make some changes to the frontmatter, not to mention that I cannot use the Obsidian logic in UTs which is a pretty big drawback that I like to avoid where possible.

• Don't use vault.delete to delete files. Use trashFile instead to make sure the file is deleted according to the users preferences. Learn more.
• Don't use the Adapter API whenever possible. Use Vault API instead. Learn more.
• Don't manage reading and write plugin data yourself. Use Plugin.loadData() and Plugin.saveData() instead.
• Do use normalizePath() if you take user defined paths. Learn more.

Performance

• Do optimize your plugin's load time. Detailed guide.
• Don't iterate all files to find a file or folder by its path. Learn more.
• If you want your plugins to be compatible with Obsidian 1.7.2+ (currently in early access), update your plugin to work with DeferredViews. Detailed guide.
• If you're using moment, make sure you're doing import { moment} from 'obsidian' so that you don't import another copy.
• Do minimize your main.js for releasing.
• Do your initial UI setup on workspace.onLayoutReady() instead of in the constructor or onload() function. Learn more.

User interface

• Don't use setting headings unless you have more than one section. Learn more.
• Don't include the word "setting" or "option" in setting headings. Learn more.
• Do use sentence case in all text in UI elements to be consistent with rest of Obsidian UI. Learn more.
• Don't use <h1> or <h2> for setting header. Use Obsidian API instead. Learn more.

I do this and can swap to the recommended way:

obsidian-linter/src/ui/linter-components/tab-components/tab.ts

Line 62 in b1cb567

this.headingEl = this.contentEl.createEl('h2', {text: nameInLanguage});

• Don't do console.log unless they are absolutely necessarily. Remove testing console logs that are not needed for production.