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

Fix parsing of ~/.nestrc in show_help_with_pager() #931

Closed
wants to merge 1 commit into from
Closed

Fix parsing of ~/.nestrc in show_help_with_pager() #931

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 20 additions & 30 deletions pynest/nest/lib/hl_api_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,37 +504,27 @@ def show_help_with_pager(hlpobj, pager):
consolepager = ['less', 'more', 'vi', 'vim', 'nano', 'emacs -nw',
'ed', 'editor']

# reading ~/.nestrc lookink for pager to use.
# check for pager command in ~/.nestrc if none provided in function call
if pager is None:
# check if .netsrc exist
rc_file = os.path.join(os.environ['HOME'], '.nestrc')
if os.path.isfile(rc_file):
# open ~/.nestrc
rc = open(rc_file, 'r')
# The loop goes through the .nestrc line by line and checks
# it for the definition of a pager. Whether a pager is
# found or not, this pager is used or the standard pager 'more'.
for line in rc:
# the re checks if there are lines beginning with '%'
rctst = re.match(r'^\s?%', line)
if rctst is None:
# the next re checks for a sth. like
# '/page << /command (more)'
# and returns the given pager.
pypagers = re.findall(
r'^\s?/page\s?<<\s?/command\s?\((\w*)', line)
if pypagers:
for pa in pypagers:
if pa:
pager = pa
else:
pager = 'more'
break
else:
pager = 'more'
rc.close()
else:
pager = 'more'
# default to `more`
pager = 'more'

# Try to open ~/.nestrc safely
try:
with open(os.path.join(os.environ['HOME'], '.nestrc'), 'r') as rc:
for line in rc:
# Go through the .nestrc line by line to check for the
# definition of a pager using a regular expression
# which matches lines of the format
# ' /page << /command (more)'
# but ignores comments (via the `(?!%)` part)
for pa in re.findall(
r'^\s?(?!%)\s?\/page\s?<<\s?\/command\s?\((\w*)',
line):
pager = pa
except:
Copy link
Contributor

Choose a reason for hiding this comment

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

PEP8 advises against bare except clauses. except IOError seems appropriate here.

Copy link
Author

Choose a reason for hiding this comment

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

True, I meant to look up the most general but still appropriate exception class but forgot to do that :)

Copy link
Contributor

Choose a reason for hiding this comment

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

For the above solution the exception class would simply be NESTError.

print('Failed to open ~/.nestrc, using default pager')
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather write "parse" than "open", since in principle an exception could occur during reading as well.

pass
Copy link
Contributor

Choose a reason for hiding this comment

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

pass is not needed here, please remove it.


Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you parsing the nestrc file manually? It is run by SLI upon startup, so you can just do something like

nest.sli_func("/page GetOptions")["command"]

to get the command in a much safer way. To have a default pager available in case nestrc does not define one, you can use

try:
    pager = nest.sli_func("/page GetOptions")["command"]
except:
    pager = 'more'

Copy link
Contributor

Choose a reason for hiding this comment

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

@jougs The parsing code was introduced over a year ago in bdc87b7 and merged with #688. I reviewed then, and as now entirely overlooked the fact that we know the parser once NEST is loaded. So @physicalist, the solution proposed by @jougs is the way to go.

# Load the helptext, check the environment
# and display the helptext in the pager.
Expand Down