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

RFE: EditorConfig support for message editor #1165

Open
scop opened this issue Sep 16, 2021 · 3 comments
Open

RFE: EditorConfig support for message editor #1165

scop opened this issue Sep 16, 2021 · 3 comments

Comments

@scop
Copy link
Contributor

scop commented Sep 16, 2021

Support for EditorConfig in the message editor would be a nice touch. This way the setting would not have to be made separately for cola, if it is being done for generic git commit message editors with .editorconfig already.

Applicable properties:

So if an .editorconfig exists in project root and has a section that matches one of filenames COMMIT_EDITMSG (note EditorConfig sections are globs) and has the one of the above properties, cola would honor those as the tab and text width for the current repository as appropriate unless explicitly set (overridden) in git current repo config as cola.tabwidth or cola.textwidth. Project .editorconfig should override global git configs for those.

It should also be noted that COMMIT_EDITMSG is not the only filename git uses for commit messages, others include MERGE_MSG and SQUASH_MSG. Supporting those in appropriate scenarios would be of course very nice, but I think just looking at COMMIT_EDITMSG and applying that for all types of commit message edits would be great and very much enough at least for starters.

I think this would be fairly straightforward to implement, the only slightly annoying detail would be how to handle this in the config UI. Perhaps disabling the respective tab/text width settings for the current repo with a tooltip "managed by .editorconfig" would be an option. Or just note existence of .editorconfig there and that configuring values will override that. Or just do nothing and document it instead :)

Ref https://pypi.org/project/EditorConfig/ for EditorConfig implementation.

@scop
Copy link
Contributor Author

scop commented Sep 16, 2021

Let me know if you find this desirable, I could implement at least some of it.

@davvid
Copy link
Member

davvid commented Sep 19, 2021

This sounds like a good idea.

My sug for implementing it would be to have a config value, default to "true", that tells cola to override the configured values with the ones provided by the editorconfig. Note: we'll have to recalculate the values whenever we switch repositories.

This would technically be a change in behavior for users that have .editorconfig in their repos, but I think overall it's better for this to be opt-out because we're effectively honoring what they've configured in the repo. true is a better default in that respect so we might as well commit to it.

cola.linebreak, cola.tabwidth, cola.textwidth and cola.expandtab would be the ones that may potentially be overridden. Can these be sparsely defined in editorconfig so that only a subset is overridden? If so, we'll have to support that.

You only mentioned max_line_length so I think a sensible approach is that if editorconifg is present and it configures a max line length then cola.linebreak will behave as if it were set to true, otherwise it'll fallback to its configured value.

That way our config stuff can remain as a fallback for when .editorconfig does not exist.

Hopefully we won't need piecemeal settings to disable each aspect ~ I'm hoping we can do with just git config cola.editorconfig false to opt-out of the overrides if we need to.

Implementation-wise, the gitcfg and prefs modules currently handle the bulk of the logic around this stuff. The gitcfg module already has hooks that make it reset its state when switching repositories. Adding the editorconfig logic adjacent to gitcfg probably makes sense since they're going to have to interact.

@scop
Copy link
Contributor Author

scop commented Sep 20, 2021

Thanks for considering!

Mentioning only max_line_length was my mistake, that's what I was most interested in. To determine what settings cola supports I took a look at the all/current repository tabs in preferences UI, and found only tab width and text width there.

I now see that there's "insert spaces instead of tabs" in the settings tab, ditto "auto-wrap lines". In my opinion these would be better placed as all+current repository level options instead of global "settings" level ones.

Anyway, I think these are the corresponding EditorConfig options to the cola settings you mentioned:

All of these can be configured separately.

I would not personally bother with a UI option to opt in/out of EditorConfig at all, and I definitely would not add per setting override options for each separately, even if adding support for this would be a change in behavior for some cases. The opt in/out option exists for users already -- they can edit .editorconfig accordingly. Adding EditorConfig settings for files named these is very unlikely to be inadvertent, and I'm not sure why the settings shouldn't apply to whatever people use to edit their commit messages. That's kind of the point of these files.

But if opt in/out options will be added, I do think that setting them to true by default (i.e. honor EditorConfig if present) is the better default, for reasons you mentioned.

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

No branches or pull requests

2 participants