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

Fixed comment and uncomment functions in file.py #24620

Merged
merged 1 commit into from Jun 12, 2015
Merged

Fixed comment and uncomment functions in file.py #24620

merged 1 commit into from Jun 12, 2015

Conversation

twangboy
Copy link
Contributor

comment and uncomment now use file.replace
-- Need someone to double check the regex stuff
Added comment/uncomment to the win_file.py
Tested with states

@twangboy twangboy added the Bug broken, incorrect, or confusing behavior label Jun 12, 2015
@twangboy
Copy link
Contributor Author

Yay! It passed lint!

@UtahDave UtahDave closed this Jun 12, 2015
@UtahDave UtahDave reopened this Jun 12, 2015
@twangboy
Copy link
Contributor Author

Fixes #24215

@jfindlay jfindlay added Platform Relates to OS, containers, platform-based utilities like FS, system based apps Execution-Module Medium Change Tests-Passed and removed Bug broken, incorrect, or confusing behavior labels Jun 12, 2015
UtahDave added a commit that referenced this pull request Jun 12, 2015
Fixed comment and uncomment functions in file.py
@UtahDave UtahDave merged commit 635121e into saltstack:2014.7 Jun 12, 2015
@twangboy twangboy added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Windows P1 Priority 1 ZRELEASED - Beryllium labels Jun 23, 2015
@twangboy twangboy added this to the Be 4 milestone Jun 23, 2015
@twangboy twangboy self-assigned this Jun 23, 2015
@twangboy twangboy modified the milestones: 2015.5.2, Be 4 Jun 23, 2015
limit=regex.lstrip('^'),
backup=backup)
pattern = '^{0}{1}'.format(char, regex.lstrip('^').rstrip('$'))
repl = "{0}".format(regex.lstrip('^').rstrip('$'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is completely wrong! You are replacing the matched pattern with the regex itself.
This can only work if you don't actually use a regular expression.

For example if you try to uncomment level = (warning|error) and the file contains # level = warning it will be replaced with level = (warning|error).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mtorromeo Thanks. I'll take a look at and see if I can figure out a better solution.

@miihael
Copy link
Contributor

miihael commented Jul 31, 2015

The change is not backward compatible: new version does not recognize "[[:space:]]*" in the beginning, just after the comment char.
For example, if regex="this is commented line", the following would not be uncommented:
# this is commented line

@twangboy twangboy modified the milestones: Be 1, 2015.5.2 Aug 1, 2015
@twangboy twangboy deleted the fix_24215 branch February 18, 2016 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Execution-Module P1 Priority 1 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Tests-Passed Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants