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

Make showSaveDialog async #16245

Merged
merged 7 commits into from Jan 9, 2018

Conversation

Projects
None yet
5 participants
@50Wliu
Member

50Wliu commented Nov 20, 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

Very similar in nature to #16229. The description in that PR essentially applies to this one as well.
async-save-dialog

Important note: This PR also removes the undocumented atom.showSaveDialog and atom.showSaveDialogSync methods. Due to a lack of free space on my laptop, I am currently unable to search to see how many packages use it. If someone could help me out with that it would be helpful to know if they should be added back and deprecated or removed outright.

Depends on atom/markdown-preview#521

50Wliu added some commits Nov 20, 2017

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented Nov 21, 2017

Thanks for doing this!

I've updated my packages in atom-package-downloader pretty recently. Here's the result of my search:

$ cd atom-package-downloader/packages
$ ag 'atom\.showSaveDialog'
Output

api-blueprint-preview/lib/api-blueprint-preview-view.coffee
269: if htmlFilePath = atom.showSaveDialogSync(filePath)

asciidoc-preview/lib/asciidoc-preview-view.coffee
239: if htmlFilePath = atom.showSaveDialogSync(filePath)

atom-api-blueprint-preview/lib/api-blueprint-preview-view.coffee
269: if htmlFilePath = atom.showSaveDialogSync(filePath)

atom-image-resize/lib/resizable-image-view.coffee
172: if imageFilePath = atom.showSaveDialogSync(filePath)

atom-jscad/standalone/overrides.js
34: var fileName = atom.showSaveDialogSync ({

atom-mermaid/lib/mermaid-view.coffee
159: return unless htmlFilePath = atom.showSaveDialogSync(filePath)

atom-minify/lib/minifier.coffee
163: filename = atom.showSaveDialogSync()

atom-rst-preview-docutils/lib/atom-rst-preview-docutils-view.coffee
266: if htmlFilePath = atom.showSaveDialogSync(filePath)

atom-scad-preview/lib/atom-scad-preview-view.js
84: var fileName = atom.showSaveDialogSync ({

draw-on-image/lib/draw-on-image.coffee
47: newItemPath = atom.showSaveDialogSync(saveOptions)

ever-notedown/lib/ever-notedown-preview-view.coffee
609: if htmlFilePath = atom.showSaveDialogSync(filePath)

ever-notedown/lib/ever-notedown.coffee
1365: if noteFilePath = atom.showSaveDialogSync(filePath)

ex-mode/lib/ex.coffee
240: fullPath = atom.showSaveDialogSync()

ex-mode/spec/ex-commands-spec.coffee
166: expect(atom.showSaveDialogSync).toHaveBeenCalled()
275: expect(atom.showSaveDialogSync).toHaveBeenCalled()

ex-mode-hb/lib/ex.coffee
235: fullPath = atom.showSaveDialogSync()

ex-mode-hb/spec/ex-commands-spec.coffee
166: expect(atom.showSaveDialogSync).toHaveBeenCalled()
275: expect(atom.showSaveDialogSync).toHaveBeenCalled()

graphviz-preview-plus/lib/graphviz-preview-plus-view.js
497: const outputFilePath = atom.showSaveDialogSync(filePath);

jscad-viewer/lib/jscad-viewer-view.js
103: var fileName = atom.showSaveDialogSync({

landau-viewer/lib/landau-viewer-view.js
103: var fileName = atom.showSaveDialogSync({

language-context-free/lib/cfdg-image-editor.coffee
114: if dstFilePath = atom.showSaveDialogSync saveFilePath

language-roff/lib/conversion.js
95: return atom.applicationDelegate.showSaveDialog({defaultPath: path});

levels/lib/workspace-manager.coffee
356: if (filePath = activeTextEditor.getPath() ? atom.showSaveDialogSync())?

levels-debugger-ruby/lib/presenter/debugger-presenter.coffee
181: saveHere = textEditor.getPath() ? atom.showSaveDialogSync()

markclip/lib/markclip.coffee
79: newItemPath = atom.applicationDelegate.showSaveDialog({

markdown-it-preview/lib/markdown-preview-view.coffee
304: if htmlFilePath = atom.showSaveDialogSync(filePath)

markdown-mindmap/lib/markdown-mindmap-view.coffee
372: if htmlFilePath = atom.showSaveDialogSync(filePath)

markdown-preview/lib/markdown-preview-view.coffee
297: if htmlFilePath = atom.showSaveDialogSync(filePath)

markdown-preview-kramdown/lib/markdown-preview-view.coffee
265: if htmlFilePath = atom.showSaveDialogSync(filePath)

markdown-preview-plus/lib/markdown-preview-view.coffee
313: if htmlFilePath = atom.showSaveDialogSync(filePath)

mscgen-preview/lib/main.coffee
128: if outputFilePath = atom.showSaveDialogSync(filePath)

mscgen-preview/lib/mscgen-preview-view.coffee
301: if outputFilePath = atom.showSaveDialogSync(filePath)

plantuml-viewer/lib/plantuml-viewer-view.js
193: var savePath = atom.showSaveDialogSync(options)

quick-query/lib/quick-query-result-view.coffee
236: filepath = atom.showSaveDialogSync()

rst-preview/lib/rst-preview-view.coffee
199: if htmlFilePath = atom.showSaveDialogSync(filePath)

rst-preview-pandoc/lib/markdown-preview-view.coffee
232: if htmlFilePath = atom.showSaveDialogSync(filePath)

sass-autocompile/lib/compiler.coffee
165: filename = atom.showSaveDialogSync()

sequence-diagram/src/diagram_view.js
83: const filename = atom.showSaveDialogSync(suggestedFilename);

state-machine-cat-preview/lib/state-machine-cat-preview-view.coffee
267: if outputFilePath = atom.showSaveDialogSync(filePath)

susave/lib/susave.coffee
43: if path = atom.applicationDelegate.showSaveDialog(params)

svg-preview/lib/svg-preview-view.js
372: if (outputFilePath = atom.showSaveDialogSync(filePath)) {

todo-show/lib/todo-view.coffee
192: if outputFilePath = atom.showSaveDialogSync(filePath.toLowerCase())

viblo/lib/markdown/markdown-preview-view.js
431: if (htmlFilePath = atom.showSaveDialogSync(filePath)) {

@50Wliu

This comment has been minimized.

Member

50Wliu commented Nov 21, 2017

Thanks! From a glance it looks like quite a few were inspired by markdown-preview's usage of atom.showSaveDialogSync 😞. atom.showSaveDialog isn't used so I'll remove that one and deprecate atom.showSaveDialogSync.

50Wliu added some commits Dec 4, 2017

@50Wliu

This comment has been minimized.

Member

50Wliu commented Dec 4, 2017

Deprecation notice added. It's not as clear as I would like it to be though. Basically if you're implementing a pane item that should have custom logic when it is saved, instead of overriding core:save-as and calling atom.showSaveDialogSync you should implement ::showSaveDialogOptions and ::saveAs on your pane item which Atom will call automatically. If you need programmatic saving of files, use Pane::saveItemAs.

@iolsen

This comment has been minimized.

Contributor

iolsen commented Jan 2, 2018

👍 we're happy with this change if you are, @50Wliu. Merge when you're ready.

50Wliu added some commits Jan 8, 2018

@50Wliu 50Wliu merged commit 26df68c into master Jan 9, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@50Wliu 50Wliu deleted the wl-async-save-dialog branch Jan 9, 2018

@as-cii as-cii referenced this pull request Jan 10, 2018

Closed

Cannot use Cmd+V to paste in Save dialogue #14145

1 of 1 task complete
@kachkaev

This comment has been minimized.

kachkaev commented Feb 15, 2018

⚠️ Looks like this PR may break packages after people upgrade to 1.24. See asciidoctor/atom-asciidoc-preview#258 and asciidoctor/atom-asciidoc-preview#259.

@50Wliu

This comment has been minimized.

Member

50Wliu commented Feb 15, 2018

@kachkaev thanks for letting me know. I think I see the problem and will create a PR to fix it.

@50Wliu

This comment has been minimized.

Member

50Wliu commented Feb 16, 2018

@kachkaev upon further investigation, I don't think this PR is the issue (though I did catch a regression - thanks for that!). This PR is only in Atom 1.25 beta and not Atom 1.24, so it must be something else causing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment