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

Add in-line config overrides via weird JSON syntax #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Vawlpe
Copy link

@Vawlpe Vawlpe commented May 30, 2023

Yes I know the code isn't clean and there's probably bugs, but I'll PR this anyway if anyone's willing to take a look :p
image

@Acylation Acylation added the category/new feature The feature will be added label May 31, 2023
@Acylation
Copy link
Owner

Acylation commented May 31, 2023

Thanks for your design and input! This will surely be an effective way to styling a single block, especially scaling and some drawing options.

About design

There are several questions

  • Is the json syntax commonly used in other code-block related plugins? (i.e. admonition, abcjs)
  • Can users easily input the configs? Maybe we can provide auto format or UI for this.
  • This will introduce a plugin specific, or package specific texts in the notes, is there a cleaner way to avoid this? For example, assign id to each block, and store the configs in a data file.

The size control of the structures is going to be imporved (#14), since we gonna set all structures sharing the same length of bonds, the final picture size will depend on the molecule. In the case of multi-line block, I would like to wrap the sturctures in a table to make it neat. If we want to apply inline settings, some small changes are needed to adapt to this.

Develop schedule

I'm refactoring the project using React, and the files are changed a lot. I'll try to integrate your code to new ones, to provide block-specified configuring ability. After adaptation and neccessary documentation, we will introduce the ability to the plugin. Thanks again!

@Vawlpe
Copy link
Author

Vawlpe commented May 31, 2023

Hi, thanks for reviewing this PR!

  • Is the json syntax commonly used in other code-block plugin

    • I'm not sure about this, i only used json because the smiles-drawer
      CTORs options param is just a JS Object, which can easily be derived from
      a json string w/ JSON.parse(...)
    • I think a more intuitive way to input the settings in text would be
      something similar to note metadata, aka a list of new-line-sepatated
      optName:: optValue lines, without all the extra quotes and commas and the
      !{...}! "wrapper", i just didn't want to think about actually parsing the
      options from the text for this initial implementation, so the use of json
      was a DX decision instead of a UX one in this case lol
  • Ease of cfg input? Autocomplete / UI?

    • This way of inputting the settings is a bit unwieldy indeed, I just
      looked for some symbol that SMILES doesn't recognize, e.g !, and made the
      code expect to find JSON between 2 of that symbol, i think it would be nice
      to have a WYSIWYG-like UI to customize these settings and auto-generate the
      JSON for whatever changes you made in the UI.
  • Plugin specific text in notes = bad, cleaner option?

    • Yes maybe simply giving an ID to each block and using a separate file
      to store the actual configs could work, but i think the option of keeping
      the cfg changes directly in the block should be kept, although not very
      clean, when e.g experimenting with the settings, or only needing to change
      one small thing or something idk

@crepppy
Copy link

crepppy commented Sep 21, 2023

Any update on this @Acylation? I think this would be a nice feature to have

@Acylation
Copy link
Owner

Hi @crepppy thanks for the follow-up! I'm figuring out how to store block-specific configs and expose UI without messing notes up, perhaps using an editor extension, requiring much more coding work. I'd like to postpone this to December because I will be busy these months with PhD application.

However, if you don't mind adding extra info in your smiles block, I could create a branch for this PR, and release a preview version where you can utilize this feature. Please let me know if this would work for you, it will come out at early Oct if you need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/new feature The feature will be added
Projects
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

3 participants