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

Carriage return (CR) handling is burdensome #45

Open
ftsfranklin opened this issue Mar 3, 2021 · 1 comment
Open

Carriage return (CR) handling is burdensome #45

ftsfranklin opened this issue Mar 3, 2021 · 1 comment

Comments

@ftsfranklin
Copy link

ftsfranklin commented Mar 3, 2021

I checkout'd and tried to run make package, and it broke because it detected CR in the sample code that I cloned. This puts unnecessary burden on the user just to test the code, because I'd have to know in the first place to clone it with non-default Git settings.

Instead, it should make copies without the carriage returns, and then build off of those.

I think the Python interpreter doesn't even care about CRs in source code.

Also, this code inefficiently reads every file, whether or not it cares about that file:

def scan_for_cr(path):
    scanfiles = ('.py', '.sh')
    for root, _, files in os.walk(path):
        for fl in files:
            with open(os.path.join(root, fl), 'rb') as f:
                if b'\r' in f.read() and [x for x in scanfiles if fl.endswith(x)]:
                    raise Exception('Carriage return (\\r) found in file %s' % (os.path.join(root, fl)))

It should at least do the scanfiles check first.

def scan_for_cr(path):
    scanfiles = ('.py', '.sh')
    for root, _, files in os.walk(path):
        for fl in files:
            if any(fl.endswith(x) for x in scanfiles):
                with open(os.path.join(root, fl), 'rb') as f:
                    if b'\r' in f.read():
                        raise Exception('Carriage return (\\r) found in file %s' % (os.path.join(root, fl)))
@Reddgum
Copy link

Reddgum commented Jul 13, 2022

Yes, I ran across this today and am just baffled why anyone would put this in there.

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