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

Improvements to search #307

Closed
wants to merge 2 commits into from
Closed

Conversation

tobiasdiez
Copy link
Member

Cleaned-up version of #162.

@simonharrer
Copy link
Contributor

Can you please remove the DONE tags in your comments?

@tobiasdiez
Copy link
Member Author

DONE 😎

@koppor
Copy link
Member

koppor commented Nov 11, 2015

Sorry for being picky at the formatting. When reading the diff, the indents don't seem to be right. Besides the content of the code, we also want to have the formatting consistent to ease newcomers to read and understand the code.

Do you use Eclipse or IntelliJ? With the recent master, gradlew cleanEclipse eclipse should generate an Eclipse config with the right indent settings.

@tobiasdiez
Copy link
Member Author

@koppor I totally agree about the formatting/indenting issues. However, the last time I run the formatter on the files resulted in many formatting changes. Is it possible that you run the formatter on the files after accepting the PR? Otherwise, my commit will be again screwed.
Moreover, yes the searched text should be always highlighted. I don't understand the purpose of the preference. Maybe this should be done in a new PR.
The split button is necessary because both sides of the button are clickable whereas for a combobox only the right is clickable.

@koppor
Copy link
Member

koppor commented Nov 12, 2015

I will discuss that with the other developers. Meanwhile, I'd like to ask whether you are aware that git gui can do line-based committing: Mark the lines to commit, right click, choose "stage lines to commit". Then, you can collect only the "important" modified lines. I would propose to do the formatting and create a new commit. Amending does not work as your PR already consists of two commits. You could use git rebase -i and then "fixup" the last two commits (DONE and formatting) into the first one. Afterwards, you have to do a git push -f ("force overwrite existing branch" in git gui). BTW: As far as I remember, it is possible to mark some lines in the code and then push "CTRL+SHIFT+f" to format. Then, the line-based commit is not necessary; only the merging of the three commits into one ("fixup" at git rebase -i).

@koppor
Copy link
Member

koppor commented Nov 12, 2015

I created a new branch uisearchhistory and merged the content there. We will continue working on the code there. Hope, that's OK!

@koppor koppor closed this Nov 12, 2015
@koppor koppor mentioned this pull request Nov 13, 2015
13 tasks
@tobiasdiez tobiasdiez deleted the uisearchhistory branch December 1, 2015 18:18
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.

3 participants