Skip to content

Profiles support, implementation of #237 - #750

Merged
rgbkrk merged 3 commits into
atom-community:masterfrom
stormherz:issue_237
Jan 25, 2016
Merged

Profiles support, implementation of #237#750
rgbkrk merged 3 commits into
atom-community:masterfrom
stormherz:issue_237

Conversation

@stormherz

Copy link
Copy Markdown
Contributor

Added profiles support as implementation of #237, made a few general refactorings and visual fixes accross the package. Profiles usage screencast:

profiles

@nixel2007

Copy link
Copy Markdown
Collaborator

Looks awesome!
make it clear please, built-in profile from grammars.coffee is also included? can it be changed and resaved?
or you need to run built-in profile by ctrl-shift-b and custom profile from your new panel?

@stormherz

Copy link
Copy Markdown
Contributor Author

Thanks! Right now profiles is just somewhat of a ScriptOptions wrapper + few additional fields like profile name, description. Built-in profile from grammars.coffee is used once, when the profile is created - to determine language of a file we're building profile for. I wasn't implementing profiles on top of built-in language profiles to make them more flexible - do not actually depend on language itself, but on a user-specific run options.
So, when there is no overrides in the profile - we just fall back to built-in grammar rules - command, arguments, etc.
Is that wrong, should I change something? For now it's pretty raw implementation, though functional, which we may change to meet our needs.

@nixel2007

Copy link
Copy Markdown
Collaborator

@stormherz i think it will be good if you add a scope selector as a filter to profiles list.
so if you edit a php files, when you open profiles panel it should show only php-configured profiles.
maybe you can add some for all scopes boolean flag if you think that all-scopes profiles will be need.

Built-in profile from grammars.coffee is used once, when the profile is created - to determine language of a file we're building profile for.

so... 1C (BSL) has this config:

  '1C (BSL)':
    'File Based':
      command: "oscript"
      args: (context) -> ['-encoding=utf-8', context.filepath]

What will profile contain if i add a -make to args?

Full profile will be:

  '1C (BSL)':
    'File Based':
      command: "oscript"
      args: (context) -> ['-encoding=utf-8', context.filepath, '-make'] 

?

or built-in args will be full overrided by -make arg?

@stormherz

Copy link
Copy Markdown
Contributor Author

i think it will be good if you add a scope selector as a filter to profiles list.

Sounds like a good idea, I'll add it to the implementation

What will profile contain if i add a -make to args?

First one, flag will be added to built-in grammar profile (must be that way, but I'll check next time I'll look into code)

@rgbkrk

rgbkrk commented Jan 23, 2016

Copy link
Copy Markdown
Member

Wow that's pretty slick!

@rgbkrk

rgbkrk commented Jan 23, 2016

Copy link
Copy Markdown
Member

Looks like me merging your other PR caused a merge conflict here. Want to rebase?

@stormherz

Copy link
Copy Markdown
Contributor Author

Yeah, I'll fix things up

@stormherz

Copy link
Copy Markdown
Contributor Author

I've fixed the conflict, though I think we should implement @nixel2007 proposal before merging:

i think it will be good if you add a scope selector as a filter to profiles list.

@rgbkrk, what do you think about it?

@rgbkrk

rgbkrk commented Jan 25, 2016

Copy link
Copy Markdown
Member

I like the idea of the scope selector!

I'm a bigger fan of merging this as is and then letting you iterate on that in a follow on though. Especially since I want to merge #752 and I'm not sure if that will trickle over to needing a rebase here.

@stormherz

Copy link
Copy Markdown
Contributor Author

Sounds good to me =)

rgbkrk added a commit that referenced this pull request Jan 25, 2016
Profiles support, implementation of #237
@rgbkrk
rgbkrk merged commit 0bbdc99 into atom-community:master Jan 25, 2016
@nixel2007

Copy link
Copy Markdown
Collaborator

any chance to ship a release with this feature?

@rgbkrk

rgbkrk commented Feb 10, 2016

Copy link
Copy Markdown
Member

Shipping now. Wonder if I should help us define a release maintenance schedule so it's not based on me being the bottleneck. I'm definitely happy if you ship too @nixel2007, though I'll push this one out now.

@nixel2007

Copy link
Copy Markdown
Collaborator

maybe you can write some check-list for shipping?

Here is my (for language package):

  • check that all issues of current milestone is closed. move some to next, if not.
  • write a changelog
  • update grammar with script (if needed)
  • commit and push changes
  • apm publish
  • close the milestone
  • draft new release through /releases page

@rgbkrk

rgbkrk commented Feb 10, 2016

Copy link
Copy Markdown
Member

We used to have this partially automated until we ran into weird issues with the git package we were using.

My current flow is:

  • Determine what the version bump is based on the PRs that were merged
  • Add to the changelog
  • Submit the patches for the changelog
  • run apm publish {versionBump}
  • add the changelog details to the releases page (though I haven't been doing this...)

At least for this project, I tend to be a bit more lax about versioning since there aren't APIs that are relied on externally - only UI changes.

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