multiple path choices for pasted image - #389
Conversation
hamidb80
changed the title
taylorhadden
left a comment
taylorhadden
left a comment
There was a problem hiding this comment.
This seems like a pretty useful feature. We should also update the documentation and the description of the wildcard in the Attachments settings.
hamidb80
changed the title
|
Notes
|
taylorhadden
left a comment
taylorhadden
left a comment
There was a problem hiding this comment.
After looking at this some more, I'm no longer thinking that a flat array of strings is the way to tackle this.
| ...getSettings().defaultPasteLocation.value.split(',').map(s => s.trim()), // paths are separated by comma (,) | ||
| '' // default fallback to root path | ||
| ] | ||
| const closestWildCardPrefix = "./**/" |
There was a problem hiding this comment.
A key problem with this approach is that this actually normally has an inverted meaning. This should technically be "any file that is a sibling or lower" by standard globstar syntax.
There are many options that we might want for finding & creating attachment folders, and they are not something easily encoded into a string. I think it would be better to have each option be an object, similar to how the Creation Rules are defined. That way we can define things like "find nearest ancestor" or "create if not found" in each option.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
honestly I had a weird feeling about this pattern too
something like that?
There was a problem hiding this comment.
This is definitely the right direction. I think following a pattern similar to Creation Rules (a list that opens an individual editor) would be even better.
|
Taylor do you think how many modes do we need? ( in settings )
|
|
why are there 2 places for settings? settings: Settings
workspaceSettings = new WritableStore<WorkspaceSettings>(null)the first one is available in |
|
it now works but the UI it's a little messy Taylor please checkout and give me feedback. do you agree with the direction? |
hamidb80
changed the title
hamidb80
changed the title
taylorhadden
left a comment
taylorhadden
left a comment
There was a problem hiding this comment.
This looks mostly like the right direction. You shouldn't need to hand over the attachment rules across the renderer/main divide though. We should make those settings accessible.
That could be something that I do separately if you prefer.
| getAllFonts(): Promise<string[]> | ||
| getAllLanguages(): Promise<string[]> | ||
| saveImageFromClipboard(contextPath: string): Promise<string> | ||
| saveImageFromClipboard(contextPath: string, attachmentRules: AttachmentRuleDefinition[]): Promise<string> |
There was a problem hiding this comment.
You shouldn't need to route the rules through here. The workspace settings should theoretically be accessible from the main process.
As I check, I can see I have a TODO about this in the main/Workspace.ts file. It uses the same DataFile concept as the renderer process, so it should be relatively straightforward.
|
|
||
| const attachmentModeDefinition: SettingDefinition<ResolveMode> = { | ||
| name: 'Creation Mode', | ||
| description: 'Determines how the new note is created.', |
There was a problem hiding this comment.
The descriptions here haven't been updated.
| patchBlockList: ['id'] | ||
| } | ||
|
|
||
| export default class AttachmentRule extends ObjectStore { |
There was a problem hiding this comment.
Should the attachment rule also define how the pasted attachment is named? Is the current naming scheme sufficient?
Having to redefine the naming template might be a pain though.
2 participants
addresses #370