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

Git commit box does not warn long messages #21144

Closed
sandy081 opened this issue Feb 22, 2017 · 9 comments · Fixed by #42206
Closed

Git commit box does not warn long messages #21144

sandy081 opened this issue Feb 22, 2017 · 9 comments · Fixed by #42206
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug git GIT issues verified Verification succeeded
Milestone

Comments

@sandy081
Copy link
Member

Testing #20999

There is a setting to enable warning for long commit messages. Even though it is enabled, there is no warning shown for long messages. Also it would be helpful to update the description of the setting about the length of the message

@koddsson
Copy link

Hey @joaomoreno! I'd very much like this feature after starting using visual studio code. Can I pick this up and submit a patch? If so, something I should keep in mind?

@joaomoreno
Copy link
Member

I admire your effort, but this won't be a simple task, since git is now an extension. You'll be limited by the extension API.

If you want to contribute to VS Code I suggest to go through the help wanted issues first.

@scottclayton
Copy link

scottclayton commented May 20, 2017

I still miss it a lot. There are a couple stupid things I do to work around this a bit though:

  1. type the message in the editor on an empty line first and see its length according to the status bar. e.g. if the cursor is at the end of the line and the status bar says Ln 15, Col49, then it should be 48 characters long.

  2. (on mac), make the font of the input box monospaced by adding css to vscode's own css (located at /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/electron-browser/workbench.main.css) and pasting this below all of the css

.scm-viewlet>.scm-editor>.monaco-inputbox {
  font-family: monospace;
}

then resizing the sidebar to make a message >50 characters wrap to the next line. Messing with the css does make a warning pop up in vscode that it's unsupported though, and it will have to be redone every time a new version of vscode is installed

@jayjun
Copy link
Contributor

jayjun commented Oct 25, 2017

atom

I like how Atom tucks a character countdown in the corner.

Yellow after 55 characters, then red after 72 characters. Negative numbers to count excess characters are handy too.

@scottclayton
Copy link

scottclayton commented Oct 25, 2017

@jayjun

You gave me an idea and it totally works on the Mac version (1.17.2)!!! (I don't know if you have a Mac, so this might only be for Mac users).

commit message length

You can add additional JavaScript to the VS Code app, so why not add some DOM-manipulation!?!

You can accomplish it using the following steps:

  • find the Visual Studio Code.app application file in your Mac's Application folder
  • right-click the app and click Show Package Contents
  • navigate to the Contents/Resources/app/out folder
  • open the paths.js file
  • add the following code to the bottom of the file:
try {
	window.addEventListener("keyup", gitCharacterCounter, false);

	function gitCharacterCounter() {
	  var mirror = document.getElementsByClassName("mirror")[0]
	  
	  if (mirror) {
	      var length = mirror.textContent.replace('Message (press Cmd+Enter to commit)', '').length
	      var titleLabel = document.getElementsByClassName("title-label")[0]
	      var titleSpan = titleLabel.getElementsByTagName('span')[0]
	      var labelBase = titleSpan.textContent.replace(/\([0-9]{0,100}\)/,'')

	      titleSpan.innerHTML = labelBase + ' (' + length + ')'
	  }
	};
}
catch (e) {
	console.log(e)
}
  • that's it! You will have to reload the current window of VS Code you're using

note: I updated the code to wrap it in a try/catch, since window might be undefined sometimes

screen shot 2017-10-24 at 9 33 39 pm

@jayjun
Copy link
Contributor

jayjun commented Oct 25, 2017

@scottclayton Wow, that’s an amazing hack!

I found the actual code responsible for those elements. InputBox listener here and getTitle() here.

I think adding a counter to the actual InputBox or surrounding elements is more acceptable since multiple SCMs could be active.

Would love to work with you to get a pull request in!

@scottclayton
Copy link

@jayjun Thanks!

I'm totally on board for working on a PR for restoring the feature—I've been missing it nearly every day since its removal.

I think implementing a running count of characters plus a warning when a certain length is exceeded—(configurable as settings for whether or not the warning happens and also what the length limit would be for the warning when exceeded)—would be a nice approach, since we would be getting the character count for free every time we check if the length exceeds the limit anyway. What do you think?

I couldn't see any active PRs for the feature, so I guess this is our time to shine.

I have no idea how people on github communicate with one another, so I've made a gist for us to discuss it if you're keen: https://gist.github.com/scottclayton/2c463746002ed2070ab1914bb7c7cafb

@jayjun
Copy link
Contributor

jayjun commented Oct 25, 2017

@scottclayton I submitted a WIP pull request. Let’s continue our discussion/hacking there!

@joaomoreno joaomoreno modified the milestones: Backlog, December 2017/January 2018 Dec 12, 2017
@joaomoreno
Copy link
Member

Fixed by #36890

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 29, 2018
@bpasero bpasero added the verified Verification succeeded label Feb 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api bug Issue identified by VS Code Team member as probable bug git GIT issues verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants