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.

Sign up for GitHub

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

Jump to bottom

Use string.Template when creating a new file #21706

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Use string.Template when creating a new file #21706

Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
83b20f5
Uses `string.Template` instead of `%`-formatting for new file templates.
sphh Jan 14, 2024
9fc35c0
Adds more time-related variables which can be used in the new file te…
sphh Jan 14, 2024
baa0359
Uses `datetime` instead of `time`.
sphh Jan 14, 2024
bb14240
Adds even more time-related variables which can be used in the new fi…
sphh Jan 14, 2024
67ea36e
Adds user's full name to the list of variables which can be used in t…
sphh Jan 14, 2024
a482acb
Removes heuristics when looking for the username.
sphh Jan 14, 2024
9f53443
Handles missing usernames gracefully.
sphh Jan 14, 2024
e354f62
Adds copyright line to new file template.
sphh Jan 14, 2024
289ef8f
Fixes NameError: name 'nameBuffer' is not defined
sphh Jan 14, 2024
5f249a6
Moves the block for creating the template variables behind the creati…
sphh Jan 14, 2024
cc34075
Adds file name and path to the list of variables which can be used in…
sphh Jan 14, 2024
0b24b4b
Adds the file to the default template.
sphh Jan 14, 2024
92f3c29
Adds project name and path to the list of variables which can be used…
sphh Jan 14, 2024
cc1af5d
Adds the project name to the default template.
sphh Jan 14, 2024
059d98f
Adds marker to position the cursor in the new file template.
sphh Jan 14, 2024
e23fdfc
Adds the cursor insertion marker to the default template.
sphh Jan 14, 2024
5e325a4
Fixes error when opening new file from project.
sphh Jan 15, 2024
f082e6f
Fixes failed tests with undo history.
sphh Jan 15, 2024
ece32b7
Undoes the last change.
sphh Jan 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
154 changes: 114 additions & 40 deletions spyder/plugins/editor/plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
import os.path as osp
from pathlib import Path
import re
import string
import sys
import time
from typing import Dict, Optional
import uuid

Expand Down Expand Up @@ -205,8 +205,19 @@ def __init__(self, parent, ignore_last_opened_files=False):
shebang = ['#!/usr/bin/env python3']
header = shebang + [
'# -*- coding: utf-8 -*-',
sphh marked this conversation as resolved.
Show resolved Hide resolved
'"""', 'Created on %(date)s', '',
'@author: %(username)s', '"""', '', '']
'"""',
'$|$',
sphh marked this conversation as resolved.
Show resolved Hide resolved
'',
'Created on ${date}',
'',
'@author: ${username}',
'@file: ${file}',
'@project: ${projectname}',
'',
'Copyright ${year} ${fullname}',
Comment on lines +214 to +217
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, I'm not sure how much value these fields really add by default, at least to the header (as opposed to in a user's own custom template)? IIRC, years ago we simplified this quite a bit because users complained that it was too long. And as to the specific fields:

  • I'm not sure in what scenario simply repeating the (original) file name is ever really helpful. Won't it just be untitledN.py at creation time when this is generated? And even if not, given its already displayed in the editor, file manager, etc. as the canonical, up to date source of truth, and is the name of the module when imported or the script when run, and likely to drift out of date if it changes, I don't see how it adds value to duplicate the information here.
  • The project name could perhaps theoretically have a small amount of utility, but only if users use projects, their project directory name is meaningful, and they don't create/change it after file creation. Furthermore, it is visible in the Spyder UI and from the file path, so it only adds anything extra in the case they share the file with other users, in which case it's unlikely to mean much anyway.
  • The copyright year duplicates the year in the created on, the name (mostly, if not completely) duplicates the username, and the copyright notice is no longer legally required for copyright protection and in many cases be inaccurate, e.g. in many jurisdictions, copyright is held by universities in many cases for the work of students and professors, and by employers for employees.

I could certainly see them making sense in some user-created templates, but I'm not seeing the case for adding them by default. Therefore:

Suggested change
'@file: ${file}',
'@project: ${projectname}',
'',
'Copyright ${year} ${fullname}',

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's to everyone's liking. That's why we have templates, don't we?

Personally I always find it easier to delete tokens than to look up, which tokens are available. That's why I added maybe to many to the default?

There are three reasons why I added ${file} and ${projectname}: Firstly, sometimes I move around or rename files inside a project or even split a project into two. If you were to do it with out using git, it is so much easier to move them back to their original place. Secondly, files published tend to appear all over the Net and you never know, where they originally come from. This information might help (mind you, it's not foolproof, because this information can be deleted). Thirdly, I once accidentally deleted a bunch of files. Recovering them gave me their contents, but not their name or place in the directory tree.

Re the copyright notice: If someone reused your file in one of his/her projects, the copyright notice often gets lost; often not of malice, but because it is cumbersome to track down the original copyright holder and insert it into the file's header. Including the copyright notice per default makes this in my view so much easier.

What do you think? I do not have a strong feeling to remove them, but they might be useful. What do the users say? You probably know that better than I do.

'"""',
'',
'']
try:
encoding.write(os.linesep.join(header), self.TEMPLATE_PATH,
'utf-8')
Expand Down Expand Up @@ -2203,45 +2214,23 @@ def new(self, fname=None, editorstack=None, text=None):
fname=None --> fname will be 'untitledXX.py' but do not create file
fname=<basestring> --> create file
"""
# If no text is provided, create default content
try:
if text is None:
default_content = True
text, enc = encoding.read(self.TEMPLATE_PATH)
enc_match = re.search(r'-*- coding: ?([a-z0-9A-Z\-]*) -*-',
text)
if enc_match:
enc = enc_match.group(1)
# Initialize template variables
# Windows
username = encoding.to_unicode_from_fs(
os.environ.get('USERNAME', ''))
# Linux, Mac OS X
if not username:
username = encoding.to_unicode_from_fs(
os.environ.get('USER', '-'))
VARS = {
'date': time.ctime(),
'username': username,
}
try:
text = text % VARS
except Exception:
pass
else:
default_content = False
enc = encoding.read(self.TEMPLATE_PATH)[1]
except (IOError, OSError):
text = ''
enc = 'utf-8'
default_content = True

CURSOR_INSERTION_POINT = '$|$'
sphh marked this conversation as resolved.
Show resolved Hide resolved
create_fname = lambda n: to_text_string(_("untitled")) + ("%d.py" % n)

# Creating editor widget
if editorstack is None:
current_es = self.get_current_editorstack()
else:
current_es = editorstack

projects = self.main.get_plugin(Plugins.Projects, error=False)
if projects and projects.get_active_project() is not None:
project_name = projects.get_active_project().get_name()
project_path = projects.get_active_project_path()
else:
project_name = ''
project_path = ''

created_from_here = fname is None
if created_from_here:
if self.untitled_num == 0:
Expand All @@ -2267,15 +2256,15 @@ def new(self, fname=None, editorstack=None, text=None):
self.untitled_num += 1
if not osp.isfile(fname):
break
basedir = getcwd_or_home()

projects = self.main.get_plugin(Plugins.Projects, error=False)
if projects and projects.get_active_project() is not None:
basedir = projects.get_active_project_path()
if project_path:
basedir = project_path
else:
c_fname = self.get_current_filename()
if c_fname is not None and c_fname != self.TEMPFILE_PATH:
basedir = osp.dirname(c_fname)
else:
basedir = getcwd_or_home()
fname = osp.abspath(osp.join(basedir, fname))
else:
# QString when triggered by a Qt signal
Expand All @@ -2284,6 +2273,85 @@ def new(self, fname=None, editorstack=None, text=None):
if index is not None and not current_es.close_file(index):
return

# If no text is provided, create default content
try:
if text is None:
default_content = True
text, enc = encoding.read(self.TEMPLATE_PATH)
enc_match = re.search(r'-*- coding: ?([a-z0-9A-Z\-]*) -*-',
text)
if enc_match:
enc = enc_match.group(1)

# Initialize template variables
if os.name == 'nt':
# Windows
import ctypes

username = encoding.to_unicode_from_fs(
os.environ.get('USERNAME', '-'))
GetUserNameEx = ctypes.windll.secur32.GetUserNameExW
name_display = 3

size = ctypes.pointer(ctypes.c_ulong(0))
GetUserNameEx(name_display, None, size)

name_buffer = ctypes.create_unicode_buffer(
size.contents.value)
GetUserNameEx(name_display, name_buffer, size)
displayname = name_buffer.value.strip()
else:
# Linux, Mac OS X
import pwd

username = encoding.to_unicode_from_fs(
os.environ.get('USER', '-'))
try:
pwd_struct = pwd.getpwnam(username)
except KeyError:
displayname = ''
else:
# Use first element in Gecos field as user's full name
displayname = pwd_struct.pw_gecos.split(',')[0].strip()
Comment on lines +2287 to +2315
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be made much simpler and more reliable by using the built-in standard mechanism for retrieving the current username, getpass.getuser(), and for the display/full name use pywin32 for Windows (since its already a Spyder dependency).

Suggested change
if os.name == 'nt':
# Windows
import ctypes
username = encoding.to_unicode_from_fs(
os.environ.get('USERNAME', '-'))
GetUserNameEx = ctypes.windll.secur32.GetUserNameExW
name_display = 3
size = ctypes.pointer(ctypes.c_ulong(0))
GetUserNameEx(name_display, None, size)
name_buffer = ctypes.create_unicode_buffer(
size.contents.value)
GetUserNameEx(name_display, name_buffer, size)
displayname = name_buffer.value.strip()
else:
# Linux, Mac OS X
import pwd
username = encoding.to_unicode_from_fs(
os.environ.get('USER', '-'))
try:
pwd_struct = pwd.getpwnam(username)
except KeyError:
displayname = ''
else:
# Use first element in Gecos field as user's full name
displayname = pwd_struct.pw_gecos.split(',')[0].strip()
username = getpass.getuser()
if os.name == 'nt':
# Windows
from win32api import GetUserNameEx
from win32com import NameDisplay
try:
displayname = GetUserNameEx(NameDisplay)
except Exception:
displayname = ''
else:
# Linux, macOS
import pwd
try:
pwd_struct = pwd.getpwnam(username)
except KeyError:
displayname = ''
else:
# Use first element in Gecos field as user's full name
displayname = pwd_struct.pw_gecos.split(',')[0].strip()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that spyder is already depending on these Windows module (I am not working on Windows myself) …

But what is the effect of importing the same object from different modules in these two lines:

                    from win32api import GetUserNameEx
                    from win32com import GetUserNameEx

As for the Linux part: I wanted to avoid to import another module, that's why I took that root.


now = datetime.now().astimezone()
now.replace(microsecond=0)
sphh marked this conversation as resolved.
Show resolved Hide resolved
VARS = {
'date': now.ctime(),
sphh marked this conversation as resolved.
Show resolved Hide resolved
'isodate': now.isoformat(),
sphh marked this conversation as resolved.
Show resolved Hide resolved
'year': now.strftime('%Y'),
'month': now.strftime('%m'),
'day': now.strftime('%d'),
'hour': now.strftime('%H'),
'minute': now.strftime('%M'),
'second': now.strftime('%S'),
'tzname': now.strftime('%Z'),
'monthname': now.strftime('%b'),
'weekday': now.strftime('%a'),
'username': username,
'fullname': displayname or 'n/a',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, I'd suggest just falling back to the username if there is no separate display name defined, as it seems much more sensible behavior for realistic use cases, and avoids the need to handle (and translate) a default.

Suggested change
'fullname': displayname or 'n/a',
'fullname': displayname or username,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also considered to use username as alternative, if displayname is not available. I decided against it, because I believed that it might come to a surprise, if one uses fullname and suddenly just the displayname is inserted. Maybe we should have username, fullname and full_or_username (or something similar)?

'file': fname,
'filename': os.path.basename(fname),
'filepath': os.path.dirname(fname),
sphh marked this conversation as resolved.
Show resolved Hide resolved
'projectname': project_name or 'n/a',
'projectpath': project_path or 'n/a'
Comment on lines +2336 to +2337
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not totally sure what to do about these in the case of no project being active. For starters, we should use proper capitalization, and calling _() will translate the string, i.e.

Suggested change
'projectname': project_name or 'n/a',
'projectpath': project_path or 'n/a'
'projectname': project_name or _('N/A'),
'projectpath': project_path or _('N/A'),

We could just fall back to getcwd_or_home(), as Spyder does for the default working dir when no project is active. i.e.:

Suggested change
'projectname': project_name or 'n/a',
'projectpath': project_path or 'n/a'
'projectname': project_name or _('N/A'),
'projectpath': project_path or getcwd_or_home(),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am used to both ways: ‘n/a’ and ‘N/A’. When working in the UK I got the feeling, that ‘n/a’ was more common. So does www.leo.org only shows ‘n/a’. On the other hand, wikipedia has an entry for ‘N/A’. I don't have any preferences here.

I prefer to show, that the file is not part of a project instead of using the current working directory or even home. Otherwise one might think, that the file is part of a project, which it is not.

}
try:
text = string.Template(text).safe_substitute(VARS)
except Exception:
pass
else:
default_content = False
enc = encoding.read(self.TEMPLATE_PATH)[1]
except (IOError, OSError):
sphh marked this conversation as resolved.
Show resolved Hide resolved
text = ''
enc = 'utf-8'
default_content = True

# Store position of first cursor insertion marker and remove marker
cursor_pos = text.find(CURSOR_INSERTION_POINT)
text = text.replace(CURSOR_INSERTION_POINT, '')

# Creating the editor widget in the first editorstack (the one that
# can't be destroyed), then cloning this editor widget in all other
# editorstacks:
Expand All @@ -2295,6 +2363,12 @@ def new(self, fname=None, editorstack=None, text=None):
self._clone_file_everywhere(finfo)
current_editor = current_es.set_current_filename(finfo.filename)
self.register_widget_shortcuts(current_editor)

# Move cursor to position of the insertion marker
cursor = current_editor.textCursor()
cursor.setPosition(cursor_pos)
current_editor.setTextCursor(cursor)

if not created_from_here:
self.save(force=True)

Expand Down