From 1a133c69d082b49fe1c363707707f6e6a5b1d4fb Mon Sep 17 00:00:00 2001
From: Jerome Reybert <jreybert@gmail.com>
Date: Wed, 25 Oct 2017 13:44:57 +0200
Subject: [PATCH] system wrappers now throw exception in case of error

rename functions:
	magit#utils#system -> magit#sys#system
	magit#utils#systemlist -> magit#sys#systemlist

exception thrown is 'shell_error'.

new functions, which do not throw an exception in case of error:
	magit#utils#system_noraise
	magit#utils#systemlist_noraise
---
 autoload/magit/git.vim   | 145 +++++++++++++++++++--------------------
 autoload/magit/sys.vim   | 119 ++++++++++++++++++++++++++++++++
 autoload/magit/utils.vim |  54 ---------------
 plugin/magit.vim         |  36 ++++++----
 4 files changed, 210 insertions(+), 144 deletions(-)
 create mode 100644 autoload/magit/sys.vim

diff --git a/autoload/magit/git.vim b/autoload/magit/git.vim
index 3a5d9de..a2ed3da 100644
--- a/autoload/magit/git.vim
+++ b/autoload/magit/git.vim
@@ -24,7 +24,7 @@ function! magit#git#get_status()
 	" we can't use git status -z here, because system doesn't make the
 	" difference between NUL and NL. -status z terminate entries with NUL,
 	" instead of NF
-	let status_list=magit#utils#systemlist(g:magit_git_cmd . " status --porcelain")
+	let status_list=magit#sys#systemlist(g:magit_git_cmd . " status --porcelain")
 	for file_status_line in status_list
 		let line_match = matchlist(file_status_line, '\(.\)\(.\) \%(.\{-\} -> \)\?"\?\(.\{-\}\)"\?$')
 		let filename = line_match[3]
@@ -34,13 +34,13 @@ function! magit#git#get_status()
 endfunction
 
 function! magit#git#get_config(conf_name, default)
-	silent! let git_result=magit#utils#strip(
-				\ magit#utils#system(g:magit_git_cmd . " config --get " . a:conf_name))
-	if ( v:shell_error != 0 )
+	try
+		silent! let git_result=magit#utils#strip(
+				\ magit#sys#system(g:magit_git_cmd . " config --get " . a:conf_name))
+	catch 'shell_error'
 		return a:default
-	else
-		return git_result
-	endif
+	endtry
+	return git_result
 endfunction
 
 " magit#git#is_work_tree: this function check that path passed as parameter is
@@ -51,11 +51,12 @@ function! magit#git#is_work_tree(path)
 	let dir = getcwd()
 	try
 		call magit#utils#chdir(a:path)
-		let top_dir=magit#utils#strip(
+		try
+			let top_dir=magit#utils#strip(
 					\ system(g:magit_git_cmd . " rev-parse --show-toplevel")) . "/"
-		if ( v:shell_error != 0 )
+		catch 'shell_error'
 			return ''
-		endif
+		endtry
 		return top_dir
 	finally
 		call magit#utils#chdir(dir)
@@ -69,15 +70,14 @@ function! magit#git#set_top_dir(path)
 	let dir = getcwd()
 	try
 		call magit#utils#chdir(a:path)
-		let top_dir=magit#utils#strip(
-					\ system(g:magit_git_cmd . " rev-parse --show-toplevel")) . "/"
-		if ( v:shell_error != 0 )
-			throw "magit: git-show-toplevel error: " . top_dir
-		endif
-		let git_dir=magit#utils#strip(system(g:magit_git_cmd . " rev-parse --git-dir")) . "/"
-		if ( v:shell_error != 0 )
-			throw "magit: git-git-dir error: " . git_dir
-		endif
+		try
+			let top_dir=magit#utils#strip(
+						\ system(g:magit_git_cmd . " rev-parse --show-toplevel")) . "/"
+			let git_dir=magit#utils#strip(system(g:magit_git_cmd . " rev-parse --git-dir")) . "/"
+		catch 'shell_error'
+			call magit#sys#print_shell_error()
+			throw 'set_top_dir_error'
+		endtry
 		let b:magit_top_dir=top_dir
 		let b:magit_git_dir=git_dir
 	finally
@@ -119,14 +119,18 @@ function! magit#git#git_diff(filename, status, mode)
 	let git_cmd=g:magit_git_cmd . " diff --no-ext-diff " . staged_flag .
 				\ " --no-color -p -U" . b:magit_diff_context .
 				\ " -- " . dev_null . " " . a:filename
-	silent let diff_list=magit#utils#systemlist(git_cmd)
-	if ( a:status != '?' && v:shell_error != 0 )
-		echohl WarningMsg
-		echom "Git error: " . string(diff_list)
-		echom "Git cmd: " . git_cmd
-		echohl None
-		throw 'diff error'
+
+	if ( a:status != '?' )
+		try
+			silent let diff_list=magit#sys#systemlist(git_cmd)
+		catch 'shell_error'
+			call magit#sys#print_shell_error()
+			throw 'diff error'
+		endtry
+	else
+		silent let diff_list=magit#sys#systemlist_noraise(git_cmd)
 	endif
+
 	if ( empty(diff_list) )
 		echohl WarningMsg
 		echom "diff command \"" . git_cmd . "\" returned nothing"
@@ -146,7 +150,7 @@ function! magit#git#sub_check(submodule, check_level)
 	let ignore_flag = ( a:check_level == 'modified' ) ?
 				\ '--ignore-submodules=untracked' : ''
 	let git_cmd=g:magit_git_cmd . " status --porcelain " . ignore_flag . " " . a:submodule
-	return ( !empty(magit#utils#systemlist(git_cmd)) )
+	return ( !empty(magit#sys#systemlist(git_cmd)) )
 endfunction
 
 " magit#git#git_sub_summary: helper function to get diff of a submodule
@@ -158,7 +162,7 @@ function! magit#git#git_sub_summary(filename, mode)
 	let staged_flag = ( a:mode == 'staged' ) ? " --cached " : " --files "
 	let git_cmd=g:magit_git_cmd . " submodule summary " . staged_flag . " HEAD "
 				\ .a:filename
-	silent let diff_list=magit#utils#systemlist(git_cmd)
+	silent let diff_list=magit#sys#systemlist(git_cmd)
 	if ( empty(diff_list) )
 		if ( a:mode == 'unstaged' )
 			if ( magit#git#sub_check(a:filename, 'modified') )
@@ -182,14 +186,12 @@ endfunction
 " param[in] filemane: it must be quoted if it contains spaces
 function! magit#git#git_add(filename)
 	let git_cmd=g:magit_git_cmd . " add --no-ignore-removal -- " . a:filename
-	silent let git_result=magit#utils#system(git_cmd)
-	if ( v:shell_error != 0 )
-		echohl WarningMsg
-		echom "Git error: " . git_result
-		echom "Git cmd: " . git_cmd
-		echohl None
+	try
+		silent let git_result=magit#sys#system(git_cmd)
+	catch 'shell_error'
+		call magit#sys#print_shell_error()
 		throw 'add error'
-	endif
+	endtry
 endfunction
 
 " magit#git#git_checkout: helper function to add a whole file
@@ -198,14 +200,12 @@ endfunction
 " param[in] filemane: it must be quoted if it contains spaces
 function! magit#git#git_checkout(filename)
 	let git_cmd=g:magit_git_cmd . " checkout -- " . a:filename
-	silent let git_result=magit#utils#system(git_cmd)
-	if ( v:shell_error != 0 )
-		echohl WarningMsg
-		echom "Git error: " . git_result
-		echom "Git cmd: " . git_cmd
-		echohl None
+	try
+		silent let git_result=magit#sys#system(git_cmd)
+	catch 'shell_error'
+		call magit#sys#print_shell_error()
 		throw 'checkout error'
-	endif
+	endtry
 endfunction
 
 " magit#git#git_reset: helper function to add a whole file
@@ -214,14 +214,12 @@ endfunction
 " param[in] filemane: it must be quoted if it contains spaces
 function! magit#git#git_reset(filename)
 	let git_cmd=g:magit_git_cmd . " reset HEAD -- " . a:filename
-	silent let git_result=magit#utils#system(git_cmd)
-	if ( v:shell_error != 0 )
-		echohl WarningMsg
-		echom "Git error: " . git_result
-		echom "Git cmd: " . git_cmd
-		echohl None
+	try
+		silent let git_result=magit#sys#system(git_cmd)
+	catch 'shell_error'
+		call magit#sys#print_shell_error()
 		throw 'reset error'
-	endif
+	endtry
 endfunction
 
 " magit#git#git_apply: helper function to stage a selection
@@ -236,16 +234,14 @@ function! magit#git#git_apply(header, selection)
 		let selection += [ '' ]
 	endif
 	let git_cmd=g:magit_git_cmd . " apply --recount --no-index --cached -"
-	silent let git_result=magit#utils#system(git_cmd, selection)
-	if ( v:shell_error != 0 )
-		echohl WarningMsg
-		echom "Git error: " . git_result
-		echom "Git cmd: " . git_cmd
+	try
+		silent let git_result=magit#sys#system(git_cmd, selection)
+	catch 'shell_error'
+		call magit#sys#print_shell_error()
 		echom "Tried to aply this"
 		echom string(selection)
-		echohl None
 		throw 'apply error'
-	endif
+	endtry
 endfunction
 
 " magit#git#git_unapply: helper function to unstage a selection
@@ -263,23 +259,22 @@ function! magit#git#git_unapply(header, selection, mode)
 	if ( selection[-1] !~ '^$' )
 		let selection += [ '' ]
 	endif
-	silent let git_result=magit#utils#system(
-		\ g:magit_git_cmd . " apply --recount --no-index " . cached_flag . " --reverse - ",
-		\ selection)
-	if ( v:shell_error != 0 )
-		echohl WarningMsg
-		echom "Git error: " . git_result
+	try
+		silent let git_result=magit#sys#system(
+			\ g:magit_git_cmd . " apply --recount --no-index " . cached_flag . " --reverse - ",
+			\ selection)
+	catch 'shell_error'
+		call magit#sys#print_shell_error()
 		echom "Tried to unaply this"
 		echom string(selection)
-		echohl None
 		throw 'unapply error'
-	endif
+	endtry
 endfunction
 
 " magit#git#submodule_status: helper function to return the submodule status
 " return submodule status
 function! magit#git#submodule_status()
-	return system(g:magit_git_cmd . " submodule status")
+	return magit#sys#system(g:magit_git_cmd . " submodule status")
 endfunction
 
 " magit#git#get_branch_name: get the branch name given a reference
@@ -287,18 +282,18 @@ endfunction
 " param[in] ref can be HEAD or a branch name
 " return branch name
 function! magit#git#get_branch_name(ref)
-	return magit#utils#strip(magit#utils#system(g:magit_git_cmd . " rev-parse --abbrev-ref " . a:ref))
+	return magit#utils#strip(magit#sys#system(g:magit_git_cmd . " rev-parse --abbrev-ref " . a:ref))
 endfunction
 
 " magit#git#get_commit_subject: get the subject of a commit (first line)
 " param[in] ref: reference, can be SHA1, brnach name or HEAD
 " return commit subject
 function! magit#git#get_commit_subject(ref)
-	silent let git_result=magit#utils#strip(magit#utils#system(g:magit_git_cmd . " show --no-patch --format=\"%s\" " . a:ref))
-	if ( v:shell_error != 0 )
+	try
+		return magit#utils#strip(magit#sys#system(g:magit_git_cmd . " show --no-patch --format=\"%s\" " . a:ref))
+	catch 'shell_error'
 		return ""
-	endif
-	return git_result
+	endtry
 endfunction
 
 " magit#git#get_remote_branch: get the branch name of the default remote, for
@@ -308,10 +303,10 @@ endfunction
 " param[in] type: type of default remote: upstream or push
 " return the remote branch name, 'none' if it has not
 function! magit#git#get_remote_branch(ref, type)
-	silent let git_result=magit#utils#strip(magit#utils#system(
-		\ g:magit_git_cmd . " rev-parse --abbrev-ref=loose " . a:ref . "@{" . a:type . "}"))
-	if ( v:shell_error != 0 )
+	try
+		return magit#utils#strip(magit#sys#system(
+			\ g:magit_git_cmd . " rev-parse --abbrev-ref=loose " . a:ref . "@{" . a:type . "}"))
+	catch 'shell_error'
 		return "none"
-	endif
-	return git_result
+	endtry
 endfunction
diff --git a/autoload/magit/sys.vim b/autoload/magit/sys.vim
new file mode 100644
index 0000000..c11bafd
--- /dev/null
+++ b/autoload/magit/sys.vim
@@ -0,0 +1,119 @@
+" ======= system wrappers =======
+" Recent vim/neovim versions introduced a new handy function, systemlist:
+"  > Same as |system()|, but returns a |List| with lines (parts of
+"  > output separated by NL) with NULs transformed into NLs.
+" In the same time, they introduced the capabilty of system to take a list as
+" parameter
+" These two new behavior are emulated if not present.
+" Moreover, v:shell_error are detected and an exception is thrown if any.
+" Matching functions, without exception raising, are available. The problem is
+" that if an error is awaited, the exception thrown discards the return value.
+
+" s:system: magit#sys#system internal, with explicit catch shell error
+" parameter
+" param[in] ...: command + optional args
+" return: command output as a string
+function! s:magit_system(...)
+	let dir = getcwd()
+	try
+		call magit#utils#chdir(magit#git#top_dir())
+		" List as system() input is since v7.4.247, it is safe to check
+		" systemlist, which is sine v7.4.248
+		if exists('*systemlist')
+			return call('system', a:000)
+		else
+			if ( a:0 == 2 )
+				if ( type(a:2) == type([]) )
+					" ouch, this one is tough: input is very very sensitive, join
+					" MUST BE done with "\n", not '\n' !!
+					let arg=join(a:2, "\n")
+				else
+					let arg=a:2
+				endif
+				return system(a:1, arg)
+			else
+				return system(a:1)
+			endif
+		endif
+	finally
+		call magit#utils#chdir(dir)
+	endtry
+endfunction
+
+" s:systemlist: magit#sys#systemlist internal, with explicit catch shell
+" error parameter
+" param[in] catch: boolean, do we throw an exception in case of shell error
+" param[in] ...: command + optional args to execute, args can be List or String
+" return: command output as a list
+function! s:magit_systemlist(...)
+	let dir = getcwd()
+	try
+		call magit#utils#chdir(magit#git#top_dir())
+		" systemlist since v7.4.248
+		if exists('*systemlist')
+			return call('systemlist', a:000)
+		else
+			return split(call('s:magit_system', a:000), '\n')
+		endif
+	finally
+		call magit#utils#chdir(dir)
+	endtry
+endfunction
+
+" magit#sys#system: wrapper for system, which only takes String as input in vim,
+" although it can take String or List input in neovim.
+" INFO: temporarly change pwd to git top directory, then restore to previous
+" pwd at the end of function
+" param[in] ...: command + optional args
+" return: command output as a string
+" throw 'shell_error' in case of shell error
+function! magit#sys#system(...)
+	let ret = call('s:magit_system', a:000)
+	if ( v:shell_error != 0 )
+		let b:magit_shell_error = string(ret)
+		let b:magit_shell_cmd = string(a:000)
+		throw 'shell_error'
+	endif
+	return ret
+endfunction
+
+" magit#sys#systemlist: wrapper for systemlist, which only exists in neovim for
+" the moment.
+" INFO: temporarly change pwd to git top directory, then restore to previous
+" pwd at the end of function
+" param[in] ...: command + optional args to execute, args can be List or String
+" return: command output as a list
+" throw 'shell_error' in case of shell error
+function! magit#sys#systemlist(...)
+	let ret = call('s:magit_systemlist', a:000)
+	if ( v:shell_error != 0 )
+		let b:magit_shell_error = string(ret)
+		let b:magit_shell_cmd = string(a:000)
+		throw 'shell_error'
+	endif
+	return ret
+endfunction
+
+" magit#sys#system_noraise: magit#sys#system alias, without error
+" exception
+" param[in] ...: command + optional args
+" return: command output as a string
+function! magit#sys#system_noraise(...)
+	return call('s:magit_system', a:000)
+endfunction
+
+" magit#sys#systemlist_noraise: magit#sys#systemlist alias, without error
+" exception
+" param[in] ...: command + optional args to execute, args can be List or String
+" return: command output as a list
+function! magit#sys#systemlist_noraise(...)
+	return call('s:magit_systemlist', a:000)
+endfunction
+
+function! magit#sys#print_shell_error()
+	echohl WarningMsg
+	echom "Shell command error"
+	echom "Cmd: " . b:magit_shell_cmd
+	echom "Error msg: " . b:magit_shell_error
+	echohl None
+endfunction
diff --git a/autoload/magit/utils.vim b/autoload/magit/utils.vim
index 2f3b008..c631bb6 100644
--- a/autoload/magit/utils.vim
+++ b/autoload/magit/utils.vim
@@ -51,60 +51,6 @@ function! magit#utils#clear_undo()
 	unlet old_undolevels
 endfunction
 
-" magit#utils#system: wrapper for system, which only takes String as input in vim,
-" although it can take String or List input in neovim.
-" INFO: temporarly change pwd to git top directory, then restore to previous
-" pwd at the end of function
-" param[in] ...: command + optional args
-" return: command output as a string
-function! magit#utils#system(...)
-	let dir = getcwd()
-	try
-		call magit#utils#chdir(magit#git#top_dir())
-		" List as system() input is since v7.4.247, it is safe to check
-		" systemlist, which is sine v7.4.248
-		if exists('*systemlist')
-			return call('system', a:000)
-		else
-			if ( a:0 == 2 )
-				if ( type(a:2) == type([]) )
-					" ouch, this one is tough: input is very very sensitive, join
-					" MUST BE done with "\n", not '\n' !!
-					let arg=join(a:2, "\n")
-				else
-					let arg=a:2
-				endif
-				return system(a:1, arg)
-			else
-				return system(a:1)
-			endif
-		endif
-	finally
-		call magit#utils#chdir(dir)
-	endtry
-endfunction
-
-" magit#utils#systemlist: wrapper for systemlist, which only exists in neovim for
-" the moment.
-" INFO: temporarly change pwd to git top directory, then restore to previous
-" pwd at the end of function
-" param[in] ...: command + optional args to execute, args can be List or String
-" return: command output as a list
-function! magit#utils#systemlist(...)
-	let dir = getcwd()
-	try
-		call magit#utils#chdir(magit#git#top_dir())
-		" systemlist since v7.4.248
-		if exists('*systemlist')
-			return call('systemlist', a:000)
-		else
-			return split(call('magit#utils#system', a:000), '\n')
-		endif
-	finally
-		call magit#utils#chdir(dir)
-	endtry
-endfunction
-
 " magit#utils#underline: helper function to underline a string
 " param[in] title: string to underline
 " return a string composed of strlen(title) '='
diff --git a/plugin/magit.vim b/plugin/magit.vim
index 0f1a975..9a87240 100644
--- a/plugin/magit.vim
+++ b/plugin/magit.vim
@@ -182,10 +182,12 @@ endfunction
 " WARNING: this function writes in file, it should only be called through
 " protected functions like magit#update_buffer
 function! s:mg_get_stashes()
-	silent! let stash_list=magit#utils#systemlist(g:magit_git_cmd . " stash list")
-	if ( v:shell_error != 0 )
-		echoerr "Git error: " . stash_list
-	endif
+	try
+		silent! let stash_list=magit#sys#systemlist(g:magit_git_cmd . " stash list")
+	catch 'shell_error'
+		call magit#sys#print_shell_error()
+		return
+	endtry
 
 	if (!empty(stash_list))
 		silent put =g:magit_sections.stash
@@ -222,10 +224,10 @@ function! s:mg_get_commit_section()
 		let git_dir=magit#git#git_dir()
 		" refresh the COMMIT_EDITMSG file
 		if ( b:magit_current_commit_mode == 'CC' )
-			silent! call magit#utils#system("GIT_EDITOR=/bin/false " .
+			silent! call magit#sys#system_noraise("GIT_EDITOR=/bin/false " .
 						\ g:magit_git_cmd . " -c commit.verbose=no commit -e 2> /dev/null")
 		elseif ( b:magit_current_commit_mode == 'CA' )
-			silent! call magit#utils#system("GIT_EDITOR=/bin/false " .
+			silent! call magit#sys#system_noraise("GIT_EDITOR=/bin/false " .
 						\ g:magit_git_cmd . " -c commit.verbose=no commit --amend -e 2> /dev/null")
 		endif
 		if ( !empty(b:magit_current_commit_msg) )
@@ -326,8 +328,13 @@ endfunction
 " return no
 function! s:mg_git_commit(mode) abort
 	if ( a:mode == 'CF' )
-		silent let git_result=magit#utils#system(g:magit_git_cmd .
-					\ " commit --amend -C HEAD")
+		try
+			silent let git_result=magit#sys#system(g:magit_git_cmd .
+						\ " commit --amend -C HEAD")
+		catch 'shell_error'
+			call magit#sys#print_shell_error()
+			echoerr "Commit fix failed"
+		endtry
 	else
 		let commit_flag=""
 		if ( a:mode != 'CA' && empty( magit#get_staged_files() ) )
@@ -358,16 +365,15 @@ function! s:mg_git_commit(mode) abort
 		endif
 		let commit_cmd=g:magit_git_cmd . " commit " . commit_flag .
 					\ " --file - "
-		silent! let git_result=magit#utils#system(commit_cmd, commit_msg)
+		try
+			silent! let git_result=magit#sys#system(commit_cmd, commit_msg)
+		catch 'shell_error'
+			call magit#sys#print_shell_error()
+			echoerr "Commit failed"
+		endtry
 		let b:magit_current_commit_mode=''
 		let b:magit_current_commit_msg=[]
 	endif
-	if ( v:shell_error != 0 )
-		echohl ErrorMsg
-		echom "Git error: " . git_result
-		echom "Git cmd: " . commit_cmd
-		echohl None
-	endif
 endfunction
 
 " s:mg_select_file_block: select the whole diff file, relative to the current
-- 
GitLab