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

Added search addons. #73

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

Added search addons. #73

wants to merge 2 commits into from

Conversation

vpillinger
Copy link

@vpillinger vpillinger commented Jan 3, 2017

Included dialog addons for pretty search dialogs. #58

@@ -23,6 +23,13 @@
<script src="node_modules/codemirror/mode/javascript/javascript.js" type="text/javascript"></script>
<script src="node_modules/codemirror/addon/edit/matchbrackets.js" type="text/javascript"></script>

<!-- Enable CodeMirror's search addons -->
<script src="node_modules/codemirror/addon/search/search.js" type="text/javascript"></script>
<script src="node_modules/codemirror/addon/search/jumpToLine.js" type="text/javascript"></script>
Copy link
Owner

Choose a reason for hiding this comment

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

it looks like this should be jump-to-line.js, I'm getting an error otherwise

Copy link
Author

@vpillinger vpillinger Jan 5, 2017

Choose a reason for hiding this comment

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

That is strange. You are right about that import, but it worked for me initially and fixing it didn't seem to change anything. So I'm not sure what the deal with that is on my local machine.

@wellsjo
Copy link
Owner

wellsjo commented Jan 4, 2017

Can we improve the UX a bit here? Searching from the command line doesn't work, for example. I don't think simply enabling CodeMirror's search is enough.

@vpillinger
Copy link
Author

I agree with you that the default code-mirror UX is pretty ugly, requiring Cmd+G to browse through results. So this PR doesn't finish the search feature, but it certainly moves it in the right direction while someone can work on improving the dialog boxes.
Also, don't know what you mean about searching from the command line. The CLI is right now only a shortcut to open a file. Why wouldn't a user just open the file then press Ctrl+F or Cmd+F to search after opening the file? I think there is a use-case here that I am not seeing.

@wellsjo
Copy link
Owner

wellsjo commented Jan 21, 2017

Hey sorry this took so long for me to get to, I was away for a while. Could you rebase and then I will check it out/merge?

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

Successfully merging this pull request may close these issues.

None yet

2 participants