git/editor.c, branch v2.32.2

config: fix leaks from git_config_get_string_const()

2020-08-14T17:52:04Z

There are two functions to get a single config string: - git_config_get_string() - git_config_get_string_const() One might naively think that the first one allocates a new string and the second one just points us to the internal configset storage. But in fact they both allocate a new copy; the second one exists only to avoid having to cast when using it with a const global which we never intend to free. The documentation for the function explains that clearly, but it seems I'm not alone in being surprised by this. Of 17 calls to the function, 13 of them leak the resulting value. We could obviously fix these by adding the appropriate free(). But it would be simpler still if we actually had a non-allocating way to get the string. There's git_config_get_value() but that doesn't quite do what we want. If the config key is present but is a boolean with no value (e.g., "[foo]bar" in the file), then we'll get NULL (whereas the string versions will print an error and die). So let's introduce a new variant, git_config_get_string_tmp(), that behaves as these callers expect. We need a new name because we have new semantics but the same function signature (so even if we converted the four remaining callers, topics in flight might be surprised). The "tmp" is because this value should only be held onto for a short time. In practice it's rare for us to clear and refresh the configset, invalidating the pointer, but hopefully the "tmp" makes callers think about the lifetime. In each of the converted cases here the value only needs to last within the local function or its immediate caller. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

real_path: remove unsafe API

2020-03-10T18:41:40Z

Returning a shared buffer invites very subtle bugs due to reentrancy or multi-threading, as demonstrated by the previous patch. There was an unfinished effort to abolish this [1]. Let's finally rid of `real_path()`, using `strbuf_realpath()` instead. This patch uses a local `strbuf` for most places where `real_path()` was previously called. However, two places return the value of `real_path()` to the caller. For them, a `static` local `strbuf` was added, effectively pushing the problem one level higher: read_gitfile_gently() get_superproject_working_tree() [1] https://lore.kernel.org/git/1480964316-99305-1-git-send-email-bmwill@google.com/ Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano

pager: add a helper function to clear the last line in the terminal

2019-06-24T20:38:46Z

There are a couple of places where we want to clear the last line on the terminal, e.g. when a progress bar line is overwritten by a shorter line, then the end of that progress line would remain visible, unless we cover it up. In 'progress.c' we did this by always appending a fixed number of space characters to the next line (even if it was not shorter than the previous), but as it turned out that fixed number was not quite large enough, see the fix in 9f1fd84e15 (progress: clear previous progress update dynamically, 2019-04-12). From then on we've been keeping track of the length of the last displayed progress line and appending the appropriate number of space characters to the next line, if necessary, but, alas, this approach turned out to be error prone, see the fix in 1aed1a5f25 (progress: avoid empty line when breaking the progress line, 2019-05-19). The next patch in this series is about to fix a case where we don't clear the last line, and on occasion do end up with such garbage at the end of the line. It would be great if we could do that without the need to deal with that without meticulously computing the necessary number of space characters. So add a helper function to clear the last line on the terminal using an ANSI escape sequence, which has the advantage to clear the whole line no matter how wide it is, even after the terminal width changed. Such an escape sequence is not available on dumb terminals, though, so in that case fall back to simply print a whole terminal width (as reported by term_columns()) worth of space characters. In 'editor.c' launch_specified_editor() already used this ANSI escape sequence, so replace it with a call to this function. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano

trace2:data: add editor/pager child classification

2019-02-22T23:27:59Z

Add trace2 process classification for editor and pager child processes. Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano

editor: add a function to launch the sequence editor

2018-08-10T18:56:22Z

As part of the rewrite of interactive rebase, the sequencer will need to open the sequence editor to allow the user to edit the todo list. Instead of duplicating the existing launch_editor() function, this refactors it to a new function, launch_specified_editor(), which takes the editor as a parameter, in addition to the path, the buffer and the environment variables. launch_sequence_editor() is then added to launch the sequence editor. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano

launch_editor(): indicate that Git waits for user input

2017-12-07T18:10:19Z

When a graphical GIT_EDITOR is spawned by a Git command that opens and waits for user input (e.g. "git rebase -i"), then the editor window might be obscured by other windows. The user might be left staring at the original Git terminal window without even realizing that s/he needs to interact with another window before Git can proceed. To this user Git appears hanging. Print a message that Git is waiting for editor input in the original terminal and get rid of it when the editor returns, if the terminal supports erasing the last line. Also, make sure that our message is terminated with a whitespace so that any message the editor may show upon starting up will be kept separate from our message. Power users might not want to see this message or their editor might already print such a message (e.g. emacsclient). Allow these users to suppress the message by disabling the "advice.waitingForEditor" config. The standard advise() function is not used here as it would always add a newline which would make deleting the message harder. Helped-by: Junio C Hamano Signed-off-by: Lars Schneider Signed-off-by: Junio C Hamano

refactor "dumb" terminal determination

2017-12-04T17:38:30Z

Move the code to detect "dumb" terminals into a single location. This avoids duplicating the terminal detection code yet again in a subsequent commit. Signed-off-by: Lars Schneider Signed-off-by: Junio C Hamano

editor.c: use error_errno()

2016-05-09T19:29:08Z

Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano

run-command: introduce CHILD_PROCESS_INIT

2014-08-20T16:53:37Z

Most struct child_process variables are cleared using memset first after declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to initialize them statically instead. That's shorter, doesn't require a function call and is slightly more readable (especially given that we already have STRBUF_INIT, ARGV_ARRAY_INIT etc.). Helped-by: Johannes Sixt Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano

editor: use canonicalized absolute path

2013-07-29T19:15:27Z

By improving the relative_path() algorithm, e02ca72 (path.c: refactor relative_path(), not only strip prefix, 2013-06-25) uncovered a latent bug in Emacs. While most editor applications like cat and vim handle non-canonicalized relative paths fine, emacs does not. This is due to a long-standing bug in emacs, where it refuses to resolve symlinks in the supplied path: #!/bin/sh cd /tmp mkdir z z/a z/b echo moodle >z/a/file ln -s z/b cd b emacs ../a/file # fail: attempts to open /tmp/a/file Even if emacs were to be patched to fix this bug, it may be nicer to help users running older versions. Note that this can potentially regress for users of all editors, when they ask "what file am I editing?" to the editor, as it is likely to answer with an unsightly long full path. Co-authored-by: Nguyễn Thái Ngọc Duy Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano