[WIP] Auto clean up url links(https://github.com/koppor/jabref/issues/254) #3394
Conversation
* Created unit test class for Formatter * Add Localized value key * Add formatter to formatter list with category OTHERS * Add URLCleanup menu item for URLEditor context menu * Add generalized interface to EditorTextArea for uder-defined paste action handeling * Add menu item with url cleanup functional to EditorMenus class * Set url cleanup on paste action for TextArea in URLEditor class * Add disable URLCleanupMenu if selected text is empty * Update CHANGELOG.md
sudoHackIn
changed the title
| String toDecode = value; | ||
|
|
||
| // This regexp find "url=" or "to=" parameter in full link and get text after them | ||
| String urlRegexp = "(?:url|to)=([^&]*)"; |
There was a problem hiding this comment.
Extract the pattern to a static Pattern and use Pattern.compile
This is more efficient, I think examples are in other integrity checker classes
| import org.jabref.logic.formatter.bibtexfields.NormalizeNamesFormatter; | ||
| import org.jabref.logic.l10n.Localization; | ||
|
|
||
| import javax.swing.*; |
There was a problem hiding this comment.
Please no general imports with a star, because that loads all classes in the package.
Our code formatter for eclipse and intellij should have the correct orders
|
|
||
| // init paste handler for URLEditor to format pasted url link in textArea | ||
| textArea.setPasteActionHandler(()->{ | ||
| textArea.setText(new CleanupURLFormatter().format(textArea.getText())); |
There was a problem hiding this comment.
If it's a one liner lambda expression, you don't need to add curly braces
| * Set pasteActionHandler variable to passed handler | ||
| * @param handler an instance of PasteActionHandler that describes paste behavior | ||
| */ | ||
| public void setPasteActionHandler(@Nonnull PasteActionHandler handler) { |
There was a problem hiding this comment.
Avoid the non null annotation and use Objects.requireNonNulll
|
|
||
| @Override | ||
| public String getDescription() { | ||
| return "Cleanup URL Link by removing special symbols and extracting simple link"; |
There was a problem hiding this comment.
Description should be localized, too
|
Thanks for your contribution, it is no longer required to rebase your committs into one, as github can Show all changes across commits and on merge we typically squash all commits into one (automatically done by github) Make sure to look at the Travis Ci log to see why the build failed, just click on it |
| @@ -23,6 +28,15 @@ public UrlEditor(String fieldName, DialogService dialogService, AutoCompleteSugg | |||
| ControlHelper.loadFXMLForControl(this); | |||
|
|
|||
| textArea.textProperty().bindBidirectional(viewModel.textProperty()); | |||
| Supplier<List<MenuItem>> contextMenuSupplier = EditorMenus.getCleanupURLMenu(textArea); | |||
| textArea.addToContextMenu(contextMenuSupplier); | |||
| MenuItem cleanupURLMenuItem = contextMenuSupplier.get().get(0); | |||
There was a problem hiding this comment.
Thats a bit ugly: you need to know that the cleanup url menu item is the first one returned. Thus the code fails if somebody changes the getCleanupURLMenu method. Everything related to the context-menu should actually go there. So much for the theory; I'm not sure if and how a menu item can be notified that it is displayed. If you don't find a good solution, I would remove the disable check for the moment.
|
@sudoHackIn Please know that closing and removing this PR is the worst outcome for all of us. I checked your GitHub account and it seems you weren't active for a long time and now you jump in solely for the purpose of helping JabRef. We highly appreciate this! I know that the language in review comments sounds sometimes harsh to the unaccustomed ear. Please, never take it personally because behind every username there is indeed a friendly human (as unbelievable as this sometimes seems). Unfortunately, we all lack one thing and this is time. That's why we try to communicate as clearly as possible which is the best way to communicate issues but clearly not the best way to communicate to a person. If you feel lost since you don't know how to respond or resolve things like
Comment on this and say that you don't have an idea how to solve this and whether someone can help or tip you off. Additionally, I'm aware that the code-style issues are sometimes a pain but if you are using IDEA, it can sort everything out for you. We have an IDEA codestyle config readily available so that you don't have to deal with e.g. Although I just skimmed over your code, I see that you have added to CHANGELOG, wrote a test, etc. So this is in my humble opinion no PR that should be closed without a very definite reason. Is there a way to convince you to comment on why you did close this? |
|
Hi @halirutan! Thank you for your kind feedback. Everything is ok) Sorry, that i confused you all with this situation. I will take all of yours remarks into consideration and again create PK. This happens just because i made so big mess in git repo with rebasing and force pushing :( So the easiest solution for me looks like try forking jabref again and repeat this changes. |
Created unit test class for Formatter
Add Localized value key
Add formatter to formatter list with category OTHERS
Add URLCleanup menu item for URLEditor context menu
Add generalized interface to EditorTextArea for uder-defined paste action handeling
Add menu item with url cleanup functional to EditorMenus class
Set url cleanup on paste action for TextArea in URLEditor class
Add disable URLCleanupMenu if selected text is empty
Update CHANGELOG.md
gradle localizationUpdate?I am currently a new person in this project and this is my first pull request ever. Could you please give me a full feedback. I have questions about logging correctness, working with new menu item add disabling menu item, if selected text is empty. Is my commit corresponds to rules? Is it correct to rebase few commits into one before pull request?
Hope you understand my questions and my tediousness)