ronantakizawa/codereview-bench huggingface dataset:
Repository: charmbracelet/bubbletea
File: keyboard.go (Go)

Code before:
package tea

import (
	"github.com/charmbracelet/x/ansi"
)

// KeyboardEnhancementsMsg is a message that gets sent when the terminal
// supports keyboard enhancements.
type KeyboardEnhancementsMsg struct {
	// Flags is a bitmask of supported keyboard enhancement features.
	// See [ansi.KittyReportEventTypes] and other constants for details.
	Flags int
}

// SupportsEventTypes returns whether the terminal supports reporting
// different types of key events (press, release, repeat).
func (k KeyboardEnhancementsMsg) SupportsEventTypes() bool {
	return k.Flags&ansi.KittyReportEventTypes != 0
}


Review comment (suggestion):
```suggestion
// SupportsEventTypes returns whether the terminal supports reporting
// different types of key events (press, release, and repeat).
```

Code after:
package tea

import (
	"github.com/charmbracelet/x/ansi"
)

// KeyboardEnhancementsMsg is a message that gets sent when the terminal
// supports keyboard enhancements.
type KeyboardEnhancementsMsg struct {
	// Flags is a bitmask of supported keyboard enhancement features.
	// See [ansi.KittyReportEventTypes] and other constants for details.
	//
	// Example:
	//
	//  ```go
	//  // The hard way
	//  if msg.Flags&ansi.KittyReportEventTypes != 0 {
	//     // Terminal supports reporting different key event types
	//  }
	//
	//  // The easy way
	//  if msg.SupportsEventTypes() {
	//     // Terminal supports reporting different key event types
	//  }
	//  ```
	Flags int
}

// SupportsEventTypes returns whether the terminal supports reporting
// different types of key events (press, release, and repeat).
func (k KeyboardEnhancementsMsg) SupportsEventTypes() bool {
	return k.Flags&ansi.KittyReportEventTypes != 0
}
Repository: chubin/cheat.sh
File: tests/run-tests.sh (Shell)

Code before:
#!/bin/bash

# 1) start server:
#   without caching:
#       REDIS_HOST=None CHEATSH_PORT=50000 python bin/srv.py
#       (recommended)
#   with caching:
#       REDIS_PREFIX=TEST1 CHEATSH_PORT=50000 python bin/srv.py 
#       (for complex search queries + to test caching)
# 2) configure CHTSH_URL
# 3) run the script

# work from script's dir
cd "$(dirname "$0")" || exit

# detect Python - if not set in env, try default virtualenv
PYTHON="${PYTHON:-../ve/bin/python}"
# if no virtalenv, try current python3 binary
if ! command -v "$PYTHON" &> /dev/null; then
  PYTHON=$(command -v python3)
fi
python_version="$($PYTHON -c 'import sys; print(sys.version_info[0])')"
echo "Using PYTHON $python_version: $PYTHON"

skip_online="${CHEATSH_TEST_SKIP_ONLINE:-NO}"
test_standalone="${CHEATSH_TEST_STANDALONE:-YES}"
show_details="${CHEATSH_TEST_SHOW_DETAILS:-YES}"

TMP=$(mktemp /tmp/cht.sh.tests-XXXXXXXXXXXXXX)
TMP2=$(mktemp /tmp/cht.sh.tests-XXXXXXXXXXXXXX)
TMP3=$(mktemp /tmp/cht.sh.tests-XXXXXXXXXXXXXX)
trap 'rm -rf $TMP $TMP2 $TMP3' EXIT

export CHTSH_URL=http://cht.sh:50000
CHTSH_SCRIPT=$(dirname "$(dirname "$(readlink -f "$0")")")/share/cht.sh.txt

export PYTHONIOENCODING=UTF-8

i=0

Review comment (suggestion):
```suggestion
cd "$(dirname "$0")" || exit 1
```

Code after:
#!/bin/bash

# 1) start server:
#   without caching:
#       CHEATSH_CACHE_TYPE=none CHEATSH_PORT=50000 python bin/srv.py
#       (recommended)
#   with caching:
#       CHEATSH_REDIS_PREFIX=TEST1 CHEATSH_PORT=50000 python bin/srv.py
#       (for complex search queries + to test caching)
# 2) configure CHTSH_URL
# 3) run the script

# work from script's dir
cd "$(dirname "$0")" || exit

# detect Python - if not set in env, try default virtualenv
PYTHON="${PYTHON:-../ve/bin/python}"
# if no virtalenv, try current python3 binary
if ! command -v "$PYTHON" &> /dev/null; then
  PYTHON=$(command -v python3)
fi
python_version="$($PYTHON -c 'import sys; print(sys.version_info[0])')"
echo "Using PYTHON $python_version: $PYTHON"

skip_online="${CHEATSH_TEST_SKIP_ONLINE:-NO}"
test_standalone="${CHEATSH_TEST_STANDALONE:-YES}"
show_details="${CHEATSH_TEST_SHOW_DETAILS:-YES}"
update_tests_results="${CHEATSH_UPDATE_TESTS_RESULTS:-NO}"
CHTSH_URL="${CHTSH_URL:-http://localhost:8002}"

TMP=$(mktemp /tmp/cht.sh.tests-XXXXXXXXXXXXXX)
TMP2=$(mktemp /tmp/cht.sh.tests-XXXXXXXXXXXXXX)
TMP3=$(mktemp /tmp/cht.sh.tests-XXXXXXXXXXXXXX)
trap 'rm -rf $TMP $TMP2 $TMP3' EXIT

CHTSH_SCRIPT=$(dirname "$(dirname "$(readlink -f "$0")")")/share/cht.sh.txt

export PYTHONIOENCODING=UTF-8
Repository: chubin/cheat.sh
File: lib/cheat_wrapper.py (Python)

Code before:
visualize it using frontends and return the result.

Exports:

    cheat_wrapper()
"""

import re
import json

from routing import get_answers, get_topics_list
from search import find_answers_by_keyword
from languages_data import LANGUAGE_ALIAS, rewrite_editor_section_name
import postprocessing

import frontend.html
import frontend.ansi

def _add_section_name(query):
    # temporary solution before we don't find a fixed one
    if ' ' not in query and '+' not in query:
        return query
    if '/' in query:
        return query

    delim = None
    if ' ' in query:
        delim = " "
    elif '+' in query:
        delim = "+"


    if delim:
        index = 0
        length = len(query)
        while index != length:

            index = query.index(delim, index) + 1

            try:
                comparison = query.index(delim, index)
            except ValueError:
                comparison = -1

            if (index != comparison and index != length):
                return "%s/%s" % (query[:index-1], query[index:])

    return query

def cheat_wrapper(query, request_options=None, output_format='ansi'):
    """

Review comment (suggestion):
`delim = None` is handled by line 24
```
    if ' ' not in query and '+' not in query:
        return query
```

Code after:
visualize it using frontends and return the result.

Exports:

    cheat_wrapper()
"""

import re
import json

from routing import get_answers, get_topics_list
from search import find_answers_by_keyword
from languages_data import LANGUAGE_ALIAS, rewrite_editor_section_name
import postprocessing

import frontend.html
import frontend.ansi

def _add_section_name(query):
    # temporary solution before we don't find a fixed one
    if ' ' not in query and '+' not in query:
        return query
    if '/' in query:
        return query
    if ' ' in query:
        delim = " "
    elif '+' in query:
        delim = "+"

    index = 0
    length = len(query)
    while index != length:

        index = query.index(delim, index) + 1

        try:
            comparison = query.index(delim, index)
        except ValueError:
            comparison = -1

        if (index != comparison and index != length):
            return "%s/%s" % (query[:index-1], query[index:])

    return query

def cheat_wrapper(query, request_options=None, output_format='ansi'):
    """
    Function that delivers cheat sheet for `query`.
    If `html` is True, the answer is formatted as HTML.
    Additional request options specified in `request_options`.
    """
Repository: claudiodangelis/qrcp
File: util.go (Go)

Code before:
	ifaces, err := net.Interfaces()
	if err != nil {
		return "", err
	}
	var candidateInterface *net.Interface
	for _, iface := range ifaces {
		if iface.Name == config.Iface {
			candidateInterface = &iface
			break
		}
	}
	if candidateInterface != nil {
		ip, err := findIP(*candidateInterface)
		if err != nil {
			return "", err
		}
		return ip, nil
	}
	fmt.Println("Choose the network interface to use (type the number):")
	var filteredIfaces []net.Interface
	var re = regexp.MustCompile(`^(veth|br\-|docker|lo|EHC|XHC|bridge|gif|stf|p2p|awdl|utun|tun|tap)`)
	for _, iface := range ifaces {
		for range re.FindAllString(iface.Name, -1) {
			continue
		}
		if iface.Name == "lo" {
			continue
		}
		filteredIfaces = append(filteredIfaces, iface)
	}
	for n, iface := range filteredIfaces {
		fmt.Printf("[%d] %s\n", n, iface.Name)
	}
	reader := bufio.NewReader(os.Stdin)
	text, _ := reader.ReadString('\n')
	index, err := strconv.Atoi(strings.Trim(text, "\n\r"))
	if err != nil {
		return "", err
	}
	if index+1 > len(filteredIfaces) {
		return "", errors.New("Wrong number")
	}
	candidateInterface = &filteredIfaces[index]
	ip, err := findIP(*candidateInterface)
	if err != nil {
		return "", err
	}
	config.Iface = candidateInterface.Name
	return ip, nil
}


Review comment (refactor):
The `lo` interface is now matched by our brand new regular expression, I think we can remove this check

Code after:
	ifaces, err := net.Interfaces()
	if err != nil {
		return "", err
	}
	var candidateInterface *net.Interface
	for _, iface := range ifaces {
		if iface.Name == config.Iface {
			candidateInterface = &iface
			break
		}
	}
	if candidateInterface != nil {
		ip, err := findIP(*candidateInterface)
		if err != nil {
			return "", err
		}
		return ip, nil
	}
	fmt.Println("Choose the network interface to use (type the number):")
	var filteredIfaces []net.Interface
	var re = regexp.MustCompile(`^(veth|br\-|docker|lo|EHC|XHC|bridge|gif|stf|p2p|awdl|utun|tun|tap)`)
	for _, iface := range ifaces {
		if re.MatchString(iface.Name) {
			continue
		}
		filteredIfaces = append(filteredIfaces, iface)
	}
	for n, iface := range filteredIfaces {
		fmt.Printf("[%d] %s\n", n, iface.Name)
	}
	reader := bufio.NewReader(os.Stdin)
	text, _ := reader.ReadString('\n')
	index, err := strconv.Atoi(strings.Trim(text, "\n\r"))
	if err != nil {
		return "", err
	}
	if index+1 > len(filteredIfaces) {
		return "", errors.New("Wrong number")
	}
	candidateInterface = &filteredIfaces[index]
	ip, err := findIP(*candidateInterface)
	if err != nil {
		return "", err
	}
	config.Iface = candidateInterface.Name
	return ip, nil
}

// shouldBeZipped returns a boolean value indicating if the
// content should be zipped or not, and an error.
// The content should be zipped if:
Repository: claudiodangelis/qrcp
File: util.go (Go)

Code before:
// dialog to choose which network interface should be used
// for the transfer
func getAddress(config *Config) (string, error) {
	ifaces, err := net.Interfaces()
	if err != nil {
		return "", err
	}
	var candidateInterface *net.Interface
	for _, iface := range ifaces {
		if iface.Name == config.Iface {
			candidateInterface = &iface
			break
		}
	}
	if candidateInterface != nil {
		ip, err := findIP(*candidateInterface)
		if err != nil {
			return "", err
		}
		return ip, nil
	}
	fmt.Println("Choose the network interface to use (type the number):")
	var filteredIfaces []net.Interface
	var re = regexp.MustCompile(`^(veth|br\-|docker)`)
	for _, iface := range ifaces {
		for range re.FindAllString(iface.Name, -1) {
			continue
		}
		if iface.Name == "lo" {
			continue
		}
		filteredIfaces = append(filteredIfaces, iface)
	}
	for n, iface := range filteredIfaces {
		fmt.Printf("[%d] %s\n", n, iface.Name)
	}
	reader := bufio.NewReader(os.Stdin)
	text, _ := reader.ReadString('\n')
	index, err := strconv.Atoi(strings.Trim(text, "\n\r"))
	if err != nil {
		return "", err
	}
	if index+1 > len(filteredIfaces) {
		return "", errors.New("Wrong number")
	}
	candidateInterface = &filteredIfaces[index]
	ip, err := findIP(*candidateInterface)
	if err != nil {
		return "", err
	}
	config.Iface = candidateInterface.Name

Review comment (suggestion):
The slice returned by re.FindAllString will only ever contain one element because a single iface.Name is being matched against and the `continue` statement will continue the `for range re.FindAllString(iface.Name, -1)` loop and not the outer `for _, iface := range ifaces` loop. Instead you want something like

`if re.MatchString(iface.Name) {`

Code after:
// dialog to choose which network interface should be used
// for the transfer
func getAddress(config *Config) (string, error) {
	ifaces, err := net.Interfaces()
	if err != nil {
		return "", err
	}
	var candidateInterface *net.Interface
	for _, iface := range ifaces {
		if iface.Name == config.Iface {
			candidateInterface = &iface
			break
		}
	}
	if candidateInterface != nil {
		ip, err := findIP(*candidateInterface)
		if err != nil {
			return "", err
		}
		return ip, nil
	}
	fmt.Println("Choose the network interface to use (type the number):")
	var filteredIfaces []net.Interface
	var re = regexp.MustCompile(`^(veth|br\-|docker|lo|EHC|XHC|bridge|gif|stf|p2p|awdl|utun|tun|tap)`)
	for _, iface := range ifaces {
		if re.MatchString(iface.Name) {
			continue
		}
		filteredIfaces = append(filteredIfaces, iface)
	}
	for n, iface := range filteredIfaces {
		fmt.Printf("[%d] %s\n", n, iface.Name)
	}
	reader := bufio.NewReader(os.Stdin)
	text, _ := reader.ReadString('\n')
	index, err := strconv.Atoi(strings.Trim(text, "\n\r"))
	if err != nil {
		return "", err
	}
	if index+1 > len(filteredIfaces) {
		return "", errors.New("Wrong number")
	}
	candidateInterface = &filteredIfaces[index]
	ip, err := findIP(*candidateInterface)
	if err != nil {
		return "", err
	}
	config.Iface = candidateInterface.Name
	return ip, nil
}
Repository: claudiodangelis/qrcp
File: main.go (Go)

Code before:
	fmt.Println("Your generated address is", generatedAddress)
	qrterminal.GenerateHalfBlock(generatedAddress, qrterminal.L, os.Stdout)

	// Define a default handler for the requests
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		attemptedSuffix := r.URL.Path[len("/"):]

		if attemptedSuffix != suffix {
			fmt.Printf("[!] unauthorized attempt with /%s\n", attemptedSuffix)
		} else {
			w.Header().Set("Content-Disposition",
				"attachment; filename="+content.Name())

			http.ServeFile(w, r, content.Path)
			if content.ShouldBeDeleted {
				if err := content.Delete(); err != nil {
					log.Println("Unable to delete the content from disk", err)
				}
			}
			if err := config.Update(); err != nil {
				log.Println("Unable to update configuration", err)
			}
			os.Exit(0)
		}
	})
	// Start a new server using the listener bound to the choosen address on a random port
	log.Fatalln(http.Serve(listener, nil))

}


Review comment (security):
Mm, why don't we just attach the handler function to `/{generated address's path}` instead of `/`? No need to check for unauthorized accesses attempts!

Code after:
	// Generate the QR code
	info("Scan the following QR to start the download.")
	info("Make sure that your smartphone is connected to the same WiFi network as this computer.")
	info("Your generated address is", generatedAddress)

	qrConfig := qrterminal.Config{
		HalfBlocks:     true,
		Level:          qrterminal.L,
		Writer:         os.Stdout,
		BlackWhiteChar: "\u001b[37m\u001b[40m\u2584\u001b[0m",
		BlackChar:      "\u001b[30m\u001b[40m\u2588\u001b[0m",
		WhiteBlackChar: "\u001b[30m\u001b[47m\u2585\u001b[0m",
		WhiteChar:      "\u001b[37m\u001b[47m\u2588\u001b[0m",
	}
	if runtime.GOOS == "windows" {
		qrConfig.HalfBlocks = false
		qrConfig.Writer = colorable.NewColorableStdout()
		qrConfig.BlackChar = qrterminal.BLACK
		qrConfig.WhiteChar = qrterminal.WHITE
	}

	qrterminal.GenerateWithConfig(generatedAddress, qrConfig)

	// Define a default handler for the requests
	route := fmt.Sprintf("/%s", randomPath)
	http.HandleFunc(route, func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Disposition",
			"attachment; filename="+content.Name())

		http.ServeFile(w, r, content.Path)
		if content.ShouldBeDeleted {
			if err := content.Delete(); err != nil {
				log.Println("Unable to delete the content from disk", err)
			}
		}
		if err := config.Update(); err != nil {
			log.Println("Unable to update configuration", err)
		}
		os.Exit(0)
	})
	// Start a new server using the listener bound to the choosen address on a random port
	log.Fatalln(http.Serve(listener, nil))

}
Repository: claudiodangelis/qrcp
File: util.go (Go)

Code before:
		if strings.HasPrefix(iface.Name, "veth") {
			continue
		}
		if strings.HasPrefix(iface.Name, "br-") {
			continue
		}
		if strings.HasPrefix(iface.Name, "docker") {
			continue
		}
		if iface.Name == "lo" {
			continue
		}
		if iface.Flags&net.FlagUp == 0 {
			continue
		}
		filteredIfaces = append(filteredIfaces, iface)
	}
	if len(filteredIfaces) == 1 {
		candidateInterface = &filteredIfaces[0]
		ip, err := findIP(*candidateInterface)
		if err != nil {
			return "", err
		}
		return ip, nil
	}
	info("Choose the network interface to use (type the number):")
	for n, iface := range filteredIfaces {
		fmt.Printf("[%d] %s\n", n, iface.Name)
	}
	reader := bufio.NewReader(os.Stdin)
	text, _ := reader.ReadString('\n')
	index, err := strconv.Atoi(strings.Trim(text, "\n\r"))
	if err != nil {
		return "", err
	}
	if index+1 > len(filteredIfaces) {
		return "", errors.New("Wrong number")
	}
	candidateInterface = &filteredIfaces[index]
	ip, err := findIP(*candidateInterface)
	if err != nil {
		return "", err
	}
	config.Iface = candidateInterface.Name
	return ip, nil
}

// shouldBeZipped returns a boolean value indicating if the
// content should be zipped or not, and an error.
// The content should be zipped if:
// 1. the user passed the `-zip` flag

Review comment (suggestion):
I think this should not be suppressed: imagine running the tool for the first time with the `-quiet` flag set, and seeing a list of network interface names: it would be very confusing!



Code after:
		if strings.HasPrefix(iface.Name, "veth") {
			continue
		}
		if strings.HasPrefix(iface.Name, "br-") {
			continue
		}
		if strings.HasPrefix(iface.Name, "docker") {
			continue
		}
		if iface.Name == "lo" {
			continue
		}
		if iface.Flags&net.FlagUp == 0 {
			continue
		}
		filteredIfaces = append(filteredIfaces, iface)
	}
	if len(filteredIfaces) == 0 {
		return "", errors.New("No network interface available.")
	}
	if len(filteredIfaces) == 1 {
		candidateInterface = &filteredIfaces[0]
		ip, err := findIP(*candidateInterface)
		if err != nil {
			return "", err
		}
		return ip, nil
	}
	fmt.Println("Choose the network interface to use (type the number):")
	for n, iface := range filteredIfaces {
		fmt.Printf("[%d] %s\n", n, iface.Name)
	}
	reader := bufio.NewReader(os.Stdin)
	text, _ := reader.ReadString('\n')
	index, err := strconv.Atoi(strings.Trim(text, "\n\r"))
	if err != nil {
		return "", err
	}
	if index+1 > len(filteredIfaces) {
		return "", errors.New("Wrong number")
	}
	candidateInterface = &filteredIfaces[index]
	ip, err := findIP(*candidateInterface)
	if err != nil {
		return "", err
	}
	config.Iface = candidateInterface.Name
	return ip, nil
}

// shouldBeZipped returns a boolean value indicating if the
Repository: claudiodangelis/qrcp
File: main.go (Go)

Code before:
package main

import (
	"flag"
	"log"
	"net"
	"net/http"
	"os"
	"runtime"

	"github.com/mattn/go-colorable"
	"github.com/mdp/qrterminal"
)

var zipFlag = flag.Bool("zip", false, "zip the contents to be transfered")
var forceFlag = flag.Bool("force", false, "ignore saved configuration")
var debugFlag = flag.Bool("debug", false, "increase verbosity")
var quietFlag = flag.Bool("quiet", false, "ignores non critical output")
var portFlag = flag.Int("port", 0, "port to bind the server to")
var receiveMode = flag.Bool("recieve", false, "allows to receive files")

func main() {
	flag.Parse()

	if len(flag.Args()) == 0 {
		log.Fatalln("At least one argument is required")

	}

	config := LoadConfig()
	if *forceFlag == true {
		config.Delete()
		config = LoadConfig()
	}

	if *portFlag > 0 {
		config.Port = *portFlag
	}

	srv, listener, generatedAddress, route, stopSignal, wg := setupHTTPServer(config)

	if *receiveMode {
		receiveFilesHTTP(generatedAddress, route, flag.Args()[0], wg, stopSignal)
	} else {
		content, err := getContent(flag.Args())

Review comment (suggestion):
1. please replace with `receiveFlag`, for consistency
2. please fix typo `recieve` -> `receive`
3. please reword the description to "receives files"

Code after:
package main

import (
	"flag"
	"log"
	"net"
	"net/http"
	"os"
	"runtime"

	"github.com/mattn/go-colorable"
	"github.com/mdp/qrterminal"
)

var zipFlag = flag.Bool("zip", false, "zip the contents to be transfered")
var forceFlag = flag.Bool("force", false, "ignore saved configuration")
var debugFlag = flag.Bool("debug", false, "increase verbosity")
var quietFlag = flag.Bool("quiet", false, "ignores non critical output")
var portFlag = flag.Int("port", 0, "port to bind the server to")
var receiveFlag = flag.Bool("receive", false, "receives files")

func main() {
	flag.Parse()

	if len(flag.Args()) == 0 {
		log.Fatalln("At least one argument is required")

	}

	config := LoadConfig()
	if *forceFlag == true {
		config.Delete()
		config = LoadConfig()
	}

	if *portFlag > 0 {
		config.Port = *portFlag
	}

	srv, listener, generatedAddress, route, stopSignal, wg := setupHTTPServer(config)

	if *receiveFlag {
		receiveFilesHTTP(generatedAddress, route, flag.Args()[0], wg, stopSignal)
	} else {
		content, err := getContent(flag.Args())
Repository: claudiodangelis/qrcp
File: qr/qr.go (Go)

Code before:
package qr

import (
	"os"
	"runtime"

	"github.com/mattn/go-colorable"
	"github.com/mdp/qrterminal"
)

// RenderString as a QR code
func RenderString(s string) {
	qrConfig := qrterminal.Config{
		HalfBlocks:     true,
		Level:          qrterminal.L,
		Writer:         os.Stdout,
		BlackWhiteChar: "\u001b[37m\u001b[40m\u2584\u001b[0m",
		BlackChar:      "\u001b[30m\u001b[40m\u2588\u001b[0m",
		WhiteBlackChar: "\u001b[30m\u001b[47m\u2585\u001b[0m",
		WhiteChar:      "\u001b[37m\u001b[47m\u2588\u001b[0m",
	}
	if runtime.GOOS == "windows" {
		qrConfig.HalfBlocks = false
		qrConfig.Writer = colorable.NewColorableStdout()
		qrConfig.BlackChar = qrterminal.BLACK
		qrConfig.WhiteChar = qrterminal.WHITE
	}

	qrterminal.GenerateWithConfig(s, qrConfig)
}


Review comment (refactor):
This is not always true, an error can be thrown for other reasons too, see: https://github.com/skip2/go-qrcode/blob/master/qrcode.go#L180

Also, this is kind of error that should make the application panic, because there is no way to anticipate it (why should we expect an URL to be too long? and why should we expect to run into a generic encoding error?) or to deal with it.

What about replacing line 11 with `panic(err)`?


Code after:
package qr

import (
	"fmt"

	"github.com/skip2/go-qrcode"
)

// RenderString as a QR code
func RenderString(s string) {
	q, err := qrcode.New(s, qrcode.Medium)
	if err != nil {
		panic(err)
	}
	fmt.Println(q.ToSmallString(false))
}