Skip to content

[Enhancement] Maintain config.ron, Add Haskell#62

Closed
kevinmatthes wants to merge 3 commits into
zee-editor:masterfrom
kevinmatthes:feature/modes-haskell
Closed

[Enhancement] Maintain config.ron, Add Haskell#62
kevinmatthes wants to merge 3 commits into
zee-editor:masterfrom
kevinmatthes:feature/modes-haskell

Conversation

@kevinmatthes

Copy link
Copy Markdown
Contributor

This Pull Request fixes typos in the comments, adds comment banners and introduces a Tree-Sitter for Haskell.

@mcobzarenco mcobzarenco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this, super useful! Adding Haskell was on my todo list too! 🙇

Not a big fan of comment boxes, see below. I had one other meta-comment re: splitting changes into multiple commits / PRs. E.g. would be easy to just merge Haskell addition and discuss style changes in a separate PR -- obviously there's a balance, but stylistic things are prone to bike shedding :-)

Comment thread zee/config/config.ron
Comment thread zee/config/config.ron
Comment thread zee/config/config.ron Outdated
Comment thread zee/config/config.ron

@mcobzarenco mcobzarenco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks! Please squash the 2nd commit and I'll merge it.

EDIT: @kevinmatthes Could you also add a line in the changelog please referencing the newly added support for Haskell 🎉

@kevinmatthes kevinmatthes force-pushed the feature/modes-haskell branch from d3feddc to a5a1666 Compare July 24, 2022 16:38
@kevinmatthes kevinmatthes marked this pull request as draft July 24, 2022 16:45
This Pull Request fixes typos in the comments, adds comment banners and introduces a Tree-Sitter for Haskell.
@kevinmatthes kevinmatthes force-pushed the feature/modes-haskell branch from d9388e1 to 359ba33 Compare July 24, 2022 17:12
@kevinmatthes kevinmatthes marked this pull request as ready for review July 24, 2022 17:14
@kevinmatthes

Copy link
Copy Markdown
Contributor Author

Squashing will cause problems with the base branch which need to be resolved. The newly added changelog line will not be accepted and the configuration will be damaged. Squashing these three commits into one is not possible, at least not for me.

You can also choose to merge by squash in order to solve the problem of multiple commits.

@kevinmatthes kevinmatthes force-pushed the feature/modes-haskell branch from fce7bc9 to ae59e0f Compare July 24, 2022 18:13
@kevinmatthes kevinmatthes marked this pull request as draft July 28, 2022 17:20
@kevinmatthes

Copy link
Copy Markdown
Contributor Author

I support @iainh's #68 and would like to wait for the results of that Pull Request before continuing with this one.

@mcobzarenco

mcobzarenco commented Jul 30, 2022

Copy link
Copy Markdown
Collaborator

On the git point,

Squashing will cause problems with the base branch which need to be resolved. The newly added changelog line will not be accepted and the configuration will be damaged. Squashing these three commits into one is not possible, at least not for me.

You should checkout latest master and git rebase interactive this branch on it, i.e git rebase -i. This will allow you to squash the commits into one, edit their messages etc.

@mcobzarenco

Copy link
Copy Markdown
Collaborator

I've created a squashed version #75 -- I may merge that one (you still appear as the author in the commit)

I support @iainh's #68 and would like to wait for the results of that Pull Request before continuing with this one.

I honestly don't see any reason to wait for as that's a major change as there's immediate value in merging this PR -- better documentation and support for Haskell 🎉

@mcobzarenco

Copy link
Copy Markdown
Collaborator

Merged in #75

@kevinmatthes kevinmatthes deleted the feature/modes-haskell branch July 31, 2022 22:22
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.

2 participants