obsidian-releases icon indicating copy to clipboard operation
obsidian-releases copied to clipboard

Add plugin: Clever Search

Open yan42685 opened this issue 1 year ago • 11 comments

I am submitting a new Community Plugin

Repo URL

Link to my plugin: https://github.com/yan42685/obsidian-clever-search

Release Checklist

  • [x] I have tested the plugin on
    • [x] Windows
    • [ ] macOS
    • [ ] Linux
    • [ ] Android (if applicable)
    • [ ] iOS (if applicable)
  • [x] My GitHub release contains all required files
    • [x] main.js
    • [x] manifest.json
    • [x] styles.css (optional)
  • [x] GitHub release name matches the exact version number specified in my manifest.json (Note: Use the exact version number, don't include a prefix v)
  • [x] The id in my manifest.json matches the id in the community-plugins.json file.
  • [x] My README.md describes the plugin's purpose and provides clear usage instructions.
  • [x] I have read the developer policies at https://docs.obsidian.md/Developer+policies, and have assessed my plugins's adherence to these policies.
  • [x] I have read the tips in https://docs.obsidian.md/Plugins/Releasing/Plugin+guidelines and have self-reviewed my plugin to avoid these common pitfalls.
  • [x] I have added a license in the LICENSE file.
  • [x] My project respects and is compatible with the original license of any code from other plugins that I'm using. I have given proper attribution to these other projects in my README.md.

yan42685 avatar Jan 19 '24 10:01 yan42685

Hello!

I found the following issues in your plugin submission

Errors:

:x: Could not parse community-plugins.json, invalid JSON. Unexpected token ] in JSON at position 360643


This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it.

github-actions[bot] avatar Jan 19 '24 10:01 github-actions[bot]

Thank you for your submission, an automated scan of your plugin code's revealed the following issues:

(Code scan skipped)


[1][2][3][4]:Casting to any should be avoided as much as possible.


Do NOT open a new PR for re-validation. Once you have pushed all of the required changes, the bot will reevaluate your PR within 6 hours. If you think some of these results are incorrect, please include /skip in your comment and the reason why you think the results are incorrect.

ObsidianReviewBot avatar Jan 20 '24 04:01 ObsidianReviewBot

/skip

The issues pointed out are very helpful for refining my code, and I have made most of the suggested modifications. The specific changes implemented are detailed in the commits that will be mentioned later. However, some of the validations don't seem entirely applicable to the current state of my plugin.

Limiting the number of console.log The console.log are primarily in the jest testing code. Additionally, other logging functions like console.debug are filtered using logLevel defined at src/utils/logger.ts, which defaults to the info log level and allows for custom log level settings. Only minimal information will be printed, avoiding interference with users and other plugin developers.

Casting to any This seems necessary in the cases you've identified. If there's a better approach, please let me know, and I will make the changes.

Assigning styles via JavaScript Here are the reasons I believe changes are not necessary at this time. However, if you think that using CSS is essential, I am open to following your guidance on this matter :)

I've provided style setting options in the Style Settings plugin, allowing users to choose their preferred effects. Also, the amount of code that needs to be changed is substantial, and restructuring it might introduce new issues.

yan42685 avatar Jan 20 '24 06:01 yan42685

Your manifest is missing the authorUrl field.

this.containerEl.style.display = "block";, this.containerEl.style.zIndex = "20";, closeButton.innerText = "✖"; This should be in the CSS file.

static sleep(ms: number) { The Obsidian API already exports this as a function.

const myRemoteDirUrl1 =, const tiktokenSourceUrl = unpkgUrl + "@dqbd/[email protected]/tiktoken_bg.wasm"; These assets should be bundled and not loaded seperately.

import { debounce } from "throttle-debounce"; The Obsidian API already has a debounce function.

new Setting(devSettingContent) This should be at the end and not in the middle of the settings.

const allAbstractFiles = getInstance(Vault).getAllLoadedFiles(); Use the getFileByPath function to check if a file exists, instead of this.

id: "cs-in-file-search-floating-window",, id: "clever-search-triggerTest",, id: "clever-search-in-file",, id: "cs-toggle-privacy-mode",, id: "clever-search-in-vault",, id: "cs-in-file-search-with-omnisearch-query", Don't prefix the command id with the plugin id, Obsidian will make sure that there are no conflicts with other plugins.

"Excluded files": "Excluded files",, "Follow Obsidian Excluded Files": "Follow Obsidian Excluded Files",, "For Development": "For Development",, "Support the Project": "Support the Project", Use sentence case in UI

joethei avatar Feb 23 '24 18:02 joethei

Your manifest is missing the authorUrl field.

this.containerEl.style.display = "block";, this.containerEl.style.zIndex = "20";, closeButton.innerText = "✖"; This should be in the CSS file.

static sleep(ms: number) { The Obsidian API already exports this as a function.

const myRemoteDirUrl1 =, const tiktokenSourceUrl = unpkgUrl + "@dqbd/[email protected]/tiktoken_bg.wasm"; These assets should be bundled and not loaded seperately.

import { debounce } from "throttle-debounce"; The Obsidian API already has a debounce function.

new Setting(devSettingContent) This should be at the end and not in the middle of the settings.

const allAbstractFiles = getInstance(Vault).getAllLoadedFiles(); Use the getFileByPath function to check if a file exists, instead of this.

id: "cs-in-file-search-floating-window",, id: "clever-search-triggerTest",, id: "clever-search-in-file",, id: "cs-toggle-privacy-mode",, id: "clever-search-in-vault",, id: "cs-in-file-search-with-omnisearch-query", Don't prefix the command id with the plugin id, Obsidian will make sure that there are no conflicts with other plugins.

"Excluded files": "Excluded files",, "Follow Obsidian Excluded Files": "Follow Obsidian Excluded Files",, "For Development": "For Development",, "Support the Project": "Support the Project", Use sentence case in UI

Thank you for your feedback 😃. I've made the changes as you suggested, but I ran into a bit of a snag. I'm not familiar with build tools, so I'm unsure how to bundle everything together. Also, it might be better to download to the userData directory, right? This way, multiple vaults can share the assets, which would save space.

yan42685 avatar Feb 24 '24 05:02 yan42685

Downloading these files is not recommended, and it will not work on all platforms. There is a plugin for esbuild that you need to add to be able to bundle .wasm files.

joethei avatar Mar 01 '24 19:03 joethei

Downloading these files is not recommended, and it will not work on all platforms. There is a plugin for esbuild that you need to add to be able to bundle .wasm files.

I agree with your point; the plugin you mentioned earlier might be esbuild-plugin-wasm. However, Clever Search also offers a semantic search feature (an optional feature which only supports Windows OS currently) , which requires the use of a total of 972MB .zip files including .exe and embedding models); it seems that ultimately, downloading is necessary. Additionally, I've noticed that a community-released plugin employs the same downloading approach as mine and is compatible with multiple platforms. You can find it here: https://github.com/aidenlx/cm-chs-patch/blob/main/src/install-guide.ts. Please consider allowing me to utilize the same method for downloading assets.

yan42685 avatar Mar 02 '24 05:03 yan42685

Hi there, as this PR has not seen any activity in the last 30 days, it will be closed in 15 days unless there are any updates.

github-actions[bot] avatar Apr 01 '24 07:04 github-actions[bot]

Hi, sorry for the late reply,

I don't see any code for Windows specifically, so it seems to me that the file get's downloaded no matter the OS. The code that is OS independent should be bundled with the plugin.

As for that other plugin, when it was initially submitted everything was bundled, they changed that later.

joethei avatar Apr 02 '24 11:04 joethei

Hi Joethei,

Transitioning from downloading to bundling would require too many code changes for me, and it's quite difficult for me given my limited abilities. I don't have confidence that I won't encounter issues during the modification process. Therefore, I plan to remove the dependency on OS-independent assets, so they won't need to be downloaded automatically. Would this approach be acceptable?

yan42685 avatar Apr 03 '24 02:04 yan42685

Hi there, as this PR has not seen any activity in the last 30 days, it will be closed in 15 days unless there are any updates.

github-actions[bot] avatar May 03 '24 07:05 github-actions[bot]

Hi there, to keep things tidy, we're closing PRs after one and a half months of inactivity. Feel free to create a new pull request when you're ready to continue. Thanks for your understanding!

github-actions[bot] avatar May 18 '24 07:05 github-actions[bot]