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

Allow server path in uri hander #245

Merged
merged 3 commits into from
Jan 27, 2019
Merged

Conversation

wacki4
Copy link
Contributor

@wacki4 wacki4 commented Jan 18, 2019

Regarding to issue #244 (The URI handler is wonderful. I would like to add functions)

@cschindl
Copy link
Collaborator

Looks good. Thank you very much for your help.

@wacki4
Copy link
Contributor Author

wacki4 commented Jan 27, 2019

Also done #239 - can reopen file from context menu, and also main config option for reopening when click on item.

@cschindl
Copy link
Collaborator

Hi @wacki4,

please create an individual pull request for each issue. Mixing several issues into one pull request makes it very difficult to release individual parts. In this case the adjustments for the URI handler are good and released. But I would have to reject the other adjustments.

If files are changed locally, atom reloads them automatically. A reload is therefore not necessary. If they are changed on the ftp server, the changes are not reloaded by atom. This was the reason for the issue. The solution would be to synchronize the files with the server. Here you have to check which file version is newer.

@wacki4
Copy link
Contributor Author

wacki4 commented Jan 27, 2019

Github automaticaly sent all of them to current pull request (Add more commits by pushing to the master branch on wacki4/ftp-remote-edit.). I will try to remember to try not make such mess. Have I to open new branch for next change?

For reload - please write a comment on issue, i would not done anything or consult You first for change. Also, If You want to synchronize with server - i think that it wont be good, what You check for that? Modify date or other i think won't be good way, and if by content - it will be downloaded each time. Where i work in a team, many times someone else is reviewing code, and forced reopening like that would be great to speed up work. Also as I think from same reason #239 was created. Most of editors have such forced reload.

Also, i've made here natural sort - it is really anoing when files/directories are ordered was as now. It will be helpful if natural ordering will be implemented. If You want, i can it make optional.

@cschindl
Copy link
Collaborator

The best practice is to open a new branch in the fork for every new feature. For each branch you can open a separate pull request. Changes and discussions can then be made and tested in the corresponding branch.

I will try to merge the first commit (URI handler) into an new seperate branch, so I can merge it into the master. For your other changes, please create new branches (sorting, reload) and open new pull requests.

@cschindl cschindl changed the base branch from master to feature/urihandler January 27, 2019 17:47
@cschindl cschindl merged commit 9996dc2 into h3imdall:feature/urihandler Jan 27, 2019
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