Skip to content

Improvements to search#307

Closed
tobiasdiez wants to merge 2 commits into
JabRef:masterfrom
tobiasdiez:uisearchhistory
Closed

Improvements to search#307
tobiasdiez wants to merge 2 commits into
JabRef:masterfrom
tobiasdiez:uisearchhistory

Conversation

@tobiasdiez

Copy link
Copy Markdown
Member

Cleaned-up version of #162.

@simonharrer

Copy link
Copy Markdown
Contributor

Can you please remove the DONE tags in your comments?

@tobiasdiez

Copy link
Copy Markdown
Member Author

DONE 😎

@koppor

koppor commented Nov 11, 2015

Copy link
Copy Markdown
Member

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
Copy Markdown
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

koppor commented Nov 12, 2015

Copy link
Copy Markdown
Member

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

koppor commented Nov 12, 2015

Copy link
Copy Markdown
Member

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