Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

[WIP] Add options to automatically open and close previews #516

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

winstliu
Copy link
Contributor

@winstliu winstliu commented Oct 28, 2017

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This is an experimental PR that adds support for automatically opening and closing Markdown Previews when Markdown files are (un)focused. Features:

  • markdown-preview.automaticallyOpenPreview will automatically open a preview pane when a Markdown file is focused.
  • markdown-preview.automaticallyClosePreview will automatically close a preview pane when its corresponding Markdown file is no longer focused.
  • markdown-preview.automaticPreviewGrammars handles the list of grammars for which a preview will automatically be opened.

Alternate Designs

Some thoughts:

  • Should the two preview options be combined into one? I see the benefits of having two separate options though.
  • Is a Set the correct data structure for holding the list of destroyed items? Is it even possible for the set to have more than one item in it before onDidChangeActivePaneItem is called?
  • I added a third option, automaticPreviewGrammars, because it turned out to be really annoying for the preview to appear when opening a new untitled tab. I'd like to keep the number of options down but I'm not sure how to work around this.

Benefits

Automatic previews!

Possible Drawbacks

When just moving over tabs it can get distracting for a new preview to appear then almost immediately disappear. I've tried to add as many early returns as possible so performance should not be impacted if these options are enabled.

Applicable Issues

Closes #423

@winstliu winstliu force-pushed the wl-automatic-preview branch from 2d1eb5d to 19331b9 Compare October 29, 2017 21:27
@nineway
Copy link

nineway commented Nov 1, 2017

I have some background on this issue as I also took it upon me and finished with it after you. If you would like I can share my tests if you want.
My thoughts are:

  • Should the two preview options be combined into one? I see the benefits of having two separate options though.

No, as you say yourself there are situations that benefit separated auto open and close previews e.g. you only work on one md file at a time and want it to stay open while you switch focus.

  • Is a Set the correct data structure for holding the list of destroyed items? Is it even possible for the set to have more than one item in it before onDidChangeActivePaneItem is called?

I think it is a correct data structure as it complies with the requirements for a destroyedItem. I guess you can have more than one item in destroyedItems, e.g. if you destroy two previews within 100ms you will have two items in it.

  • I added a third option, automaticPreviewGrammars, because it turned out to be really annoying for the preview to appear when opening a new untitled tab. I'd like to keep the number of options down but I'm not sure how to work around this.

I think this is a great way of avoiding the untitled tab and making it more modular if you want some specific file to be opened.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Open preview automatically
2 participants