Solved starter bug: Issue #1976 - Rename on Second Click #2356
Conversation
ghost
assigned 
| _selectCurrentDocument(); | ||
| result = (new $.Deferred()).resolve().promise(); | ||
| if (rename) { | ||
| result = CommandManager.execute(Commands.FILE_RENAME); |
There was a problem hiding this comment.
Gabriel,
Thanks for submitting contributions to Brackets, but this code doesn't belong in the openAndSelectDocument() function. This will change the entire meaning of the function.
In general, the code should work like this: when a file is clicked in either the working set list or the project tree, and it is not already selected, the openAndSelectDocument() function should be called. If the file is already selected, then code should be called to rename it.
Also note that your code currently only works on second click in the working set, but I think it should also work in the project tree.
Not sure how familiar you are with github, but after you have updated your code, you don't need to submit a new pull request -- just push you're changes to this same branch and they will show up here. Also, add a comment to the pull request to say you are "ready for another review" so I will get notified with an e-mail.
Thanks,
Randy
There was a problem hiding this comment.
Randy,
Be welcome. It's really a pleasure contribute to Brackets. I'm sorry that i changed it context, but I was confused because of that call of openAndSelectDocument() at drop function on WorkspaceSetView module. I thought that it was only to a drag and drop event. Now, I fixed it.
I just think it's a bit confusing have a rename second click event on project tree, it already have a double click event, that is add the selected file to Workspace Set. I can implement it to project tree, but how I said to me it's a bit confusing have two effects and a same gesture to project tree. Let me know if you decide that is better implement it to project tree too.
I'm ready for another review.
Thanks,
Gabriel
Remove it from FileViewController.openAndSelectDocument method and add it to WorkingSetView drop event.
| CommandManager.execute(Commands.FILE_RENAME); | ||
| } else { | ||
| FileViewController.openAndSelectDocument($listItem.data(_FILE_KEY).fullPath, FileViewController.WORKING_SET_VIEW); | ||
| } |
There was a problem hiding this comment.
Indenting is not correct. Brackets uses indenting of 4 spaces.
Be sure that View > Enable JSLint is turned on for Brackets code -- I think that would have caught this problem.
There was a problem hiding this comment.
It doesn't point in JSLint if I didn't mixed both in a single line and it happened because I forgot to change between projects.
That's is a issue in Brackets, it doesn't keep your indenting by project. I commited again solving this indenting problem.
|
This is much better. Just need to fix the indenting. I see your point about single-click in the project tree -- it would only make sense if the file is already in the Working Set. We can add that later if desired. |
Fixing indenting.
|
Looks good. Merging. |
Solved starter bug: Issue #1976 - Rename on Second Click
It's my first attempt to solve a bug in a open source project. Although I have read all wiki, I may do something wrong. So please let me know if I did something wrong.