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

Add RecentFiles module #237

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

diluculo
Copy link
Contributor

@diluculo diluculo commented Aug 25, 2016

#215 is now separated into 4 independent PRs:

  1. Recent files module added
  2. Save all command added
  3. PersistedDocument and CodeEditor modified
  4. Korean language added

This is the first part.

@diluculo
Copy link
Contributor Author

diluculo commented Aug 25, 2016

@EchterAgo I don't find an easy way to split up a big branch into small independent ones, so I started to reinvent wheels, Sorry and thanks. All are almost identical to your cleanup branch except two things:

(1) in OpenRecentFileCommandHandler:

    public async Task Run(Command command)
    {
       ... 
        _shell.OpenDocument(await OpenFileCommandHandler.GetEditor(newPath));
       ... 
    }

(2) in PersistedDocument:

     public override async void CanClose(System.Action<bool> callback)
     {
       ... 
     }

For your reference, a merge of 4 PRs is https://github.com/diluculo/gemini/tree/215_Merged.

@EchterAgo
Copy link
Contributor

Yea, you can't always split up everything into independent units, I have the same problem. In these cases I either waited until the first part was merged and then submitted another pull request, or I included the first commits into the second pull request and rebased upon merge. BTW, I'm in Seoul right now doing a project involving Gemini, so your Korean language support is very appreciated 👍

@diluculo diluculo mentioned this pull request Sep 5, 2016
@nesterenko-kv
Copy link
Contributor

nesterenko-kv commented Oct 18, 2016

@diluculo, in VS there is no limit for recent projects.
Limit is only for MainMenu items (only 10): image
But on Home view we have all of them: image

@diluculo
Copy link
Contributor Author

@nestquik Thanks. XML serialization may be better than StringCollection.

I made a cleanup of your PR, rebased on master:
https://github.com/diluculo/gemini/tree/215_RecentFiles_cleanup

By the way, your PR shows a couple of NotifyOfPropertyChange(() => Items) lines on RecentFileViewModel.cs. Are the lines necessary?

@nesterenko-kv
Copy link
Contributor

Thanks. I'm newbie on git, so I can make mistakes.

About notifypropertychanged - it should notify on change, doesn't it?

I'm trying to build home view module like in VS at this moment.

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