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

Allow Project Find Results to be copied to clipboard#764

Open
liuderchi wants to merge 1 commit intoatom:masterfrom
liuderchi:copy_porject_search_result
Open

Allow Project Find Results to be copied to clipboard#764
liuderchi wants to merge 1 commit intoatom:masterfrom
liuderchi:copy_porject_search_result

Conversation

@liuderchi
Copy link

@liuderchi liuderchi commented Aug 4, 2016 •
edited
Loading

Related issue #416.
copy result of Project-Find to clipboard.

changes:

  1. add Util.copySearchResultFromPane() to copy text from search result pane
  2. add context menu item supporting right-click on project-find result page.
    (of course can use it via command-palette)

demo:
demo

NOTE: hidden entries under collapsed item are not copied to clipboard so that result is consistent with view.

winstliu
winstliu reviewed Aug 4, 2016
View reviewed changes
lib/project-find-view.coffee Outdated

copySearchResultFromPane: ->
atom.clipboard.write(Util.parseSearchResult())
atom.notifications.addInfo('Search results are copied to clipboard')
Copy link
Contributor

@winstliu winstliu Aug 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Search results have been copied to clipboard

@winstliu
Copy link
Contributor

winstliu commented Aug 4, 2016

Very nice! Left you a few comments to resolve the lint errors.

winstliu
winstliu reviewed Aug 18, 2016
View reviewed changes
lib/project-find-view.coffee Outdated
'project-find:replace-all': => @replaceAll()

@subscriptions.add atom.commands.add 'div.preview-pane',
'project-find:copy-search-result': @copySearchResultFromPane
Copy link
Contributor

@winstliu winstliu Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-results

Copy link
Author

@liuderchi liuderchi Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update this in menus/find-and-replace.cson also.

@liuderchi
Copy link
Author

liuderchi commented Oct 7, 2016

@50Wliu is there anything else I need to change? 😃

@winstliu
Copy link
Contributor

winstliu commented Oct 9, 2016

/cc @maxbrunsfeld

@liuderchi liuderchi force-pushed the copy_porject_search_result branch from 0be3188 to 64e8a5c Compare November 22, 2016 05:53
jerone
jerone reviewed Nov 22, 2016
View reviewed changes
lib/project/util.coffee Outdated
preview = $('span.preview', this).text()
searchResult.push '\t' + lineNumber + '\t' + preview
searchResult.push ''
searchResult.join('\n')
Copy link

@jerone jerone Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above part could be rewritten with plain DOM API to remove the space-pen dependency with minimal effort. Might help with getting this merged.

Copy link
Author

@liuderchi liuderchi Nov 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jerone that's good Idea and I will work on it.

@liuderchi
Copy link
Author

liuderchi commented Nov 27, 2016 •
edited
Loading

@50Wliu how do think about the format of copied content?

Also I guess spec for this feature is a nice-to-have.

@liuderchi liuderchi force-pushed the copy_porject_search_result branch 2 times, most recently from 24d2f35 to 9cd6ca7 Compare November 27, 2016 09:16
@hayesr hayesr mentioned this pull request Jan 25, 2017
Open
1 task
@liuderchi liuderchi force-pushed the copy_porject_search_result branch from 1d5b38c to a6c9eb9 Compare February 4, 2017 16:23
@liuderchi
Copy link
Author

liuderchi commented Feb 4, 2017 •
edited
Loading

@50Wliu @jerone this PR now is

  • using DOM API without space-pen dependency
  • having corresponding spec

Anything else I can help?

@winstliu
Copy link
Contributor

winstliu commented Feb 5, 2017

I don't feel qualified to review this PR, but I'll try to get someone else to take a look at it.

@winstliu winstliu requested a review from maxbrunsfeld February 5, 2017 05:29
@dezman
Copy link

dezman commented Mar 15, 2017 •
edited
Loading

Who has context top review this? Would love this feature.

@liuderchi liuderchi force-pushed the copy_porject_search_result branch from a6c9eb9 to 8aa9601 Compare May 2, 2017 15:42
@liuderchi
🎨 copy search result of project-find to clipboard
4841893 
- add parseSearchResult() in util.coffee with DOM api
- add copySearchResultFromPane() in project-find-view.js
- add spec in project-find-view-spec.js
- add context-menu item
@liuderchi liuderchi force-pushed the copy_porject_search_result branch from 8aa9601 to 4841893 Compare May 3, 2017 14:15
@liuderchi
Copy link
Author

liuderchi commented May 4, 2017 •
edited
Loading

currently project-find results page shows only visible items in DOM tree.
(when you scroll down, first DOM node disappears in the top, new one comes in the bottom.)

so this PR cannot parse all search results from DOM as long as search results exceed the page view.

@liuderchi
Copy link
Author

liuderchi commented Sep 14, 2017

@steveoh too bad this PR does not fit the view in current version.

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

Reviewers

@maxbrunsfeld maxbrunsfeld Awaiting requested review from maxbrunsfeld

1 more reviewer

@jerone jerone jerone left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@liuderchi @winstliu @dezman @jerone